5

I'm creating a simple ORM in Zend Framework, to roughly encapsulate a public library application, using the DbTable/Mapper/Model approach. I'm not sure if the way I'm doing my User-related classes is right, though, as I have some logic in Mapper_User, and some in Model_User.

Mapper_User

<?php
class Mapper_Users {

/*
createModelObject would be called by a Controller handling a Form_Regsiter's
data, to create a new Model_User object. This object'd then be saved by the
same Controller by calling Mapper_Users->save();
*/
    public function createModelObject(array $fields) {
        if(!isset($fields['date_registered']))
            $fields['date_registered'] = date('Y-m-d H:i:s');
        if(!isset($fields['max_concurrent_rentals']))
            $fields['max_concurrent_rentals'] = 3;
        return new Model_User($fields);
    }
}
?>

In the method which creates new Model_User objects from scratch (as in, not pulling a record from the DB, but registering a new user), I instantiate a new Model_User with the name/username/password provided from a Form, then set a few object properties such as the registration date, "max books allowed at one time" and such. This data, being stuffed inside the Model_User by the Mapper_User, then gets written to the DB when Mapper_User->save(); gets called. The Mapper feels like the right place for this to go - keeps the Model light.

Is this right, or should default fields like this be set inside Model_User itself?

Model_User

<?php
class Model_User {

    public function setPassword($value) {
        $this->password = md5($value);
    }
}
?>

When setting a user object's password, I'm doing this in Model_User->setPassword($value);, as you might expect, and doing $this->password = md5($value); inside this method. Again, this feels right - trying to do the md5 step in Mapper_User->save(); method would cause issues if the Model_User were one pulled from the DB, as the password field would clearly already be hashed.

And this is where my confusion's arising. To my mind, all the logic pertaining to "fields to do with a user" should either live in its Model, or its Mapper, but here I have some logic (default fields) in the Mapper, and some (field operations) in the Model. Is this right, or should I be trying to somehow get default fields in the Model, or field operations in the Mapper?

Thanks for taking the time to read this!


Edit for @RockyFord:

Mapper_User actually extends an Abstract I've written, as I don't like writing the same basic code in 500 Mapper_*.php files, so there's some bureaucracy due to that, but its effective __construct() is pretty simple:

<?php
class Mapper_Users {

    public function __construct() {
        $this->_db = new DbTable_Users();
        if(!$this->_db instanceof Zend_Db_Table_Abstract)
            throw new Exception('Invalid table data gateway provided');
    }
}
?>
  • it would help if you posted some code to demonstrate, I think I get it but I'm not sure. – RockyFord Sep 09 '12 at 14:03
  • @RockyFord Figured it was tidier without, but I'll add some! – Steve Griffiths Sep 09 '12 at 14:18
  • I'm trying hard to frame an answer, but I have no real idea of where to begin without understanding how you're thinking and coding. – RockyFord Sep 09 '12 at 14:20
  • Thanks @RockyFord, I really appreciate this - have added some code samples of the two code blocks in question, does this help get across what I'm doing right/wrong? – Steve Griffiths Sep 09 '12 at 14:27
  • I'll tell you right now that hashing in setPassword() isn't going to work for long. How do you __construct() your mapper? – RockyFord Sep 09 '12 at 14:29
  • @RockyFord Have added that now - would you be able to expand on why it's a bad idea having that hashing in the Model, or where it *should* go? Been doing PHP for many years but there's so many conventions with where things should go in these formalised framework structures I'm just trying to adapt to them all. – Steve Griffiths Sep 09 '12 at 14:40
  • if you need to see where I'm coming from, see me on [Github](https://github.com/jschoenwolf/home-local.git) – RockyFord Sep 09 '12 at 16:11
  • Checking out your project @RockyFord that's ace, thanks! Seems you have your Video model doing actual legwork, calling other Mappers to "save" things - didn't know Models were allowed to do such things! I think I need to do a lot more reading... – Steve Griffiths Sep 09 '12 at 17:47
  • Be careful that particular model is not a domain model, in fact it's not much more then temp code used to populate the database. The domain model is down in the module. – RockyFord Sep 10 '12 at 00:48
  • Well @RockyFord and Keyne I'm not yet sure how "reputation scores" or any such thing work here, so don't know if I should mark both your contributions as "correct answer" or what, but you've both been very helpful in me figuring this stuff out :) – Steve Griffiths Sep 15 '12 at 10:35
  • Well RockyFord and @Keyne I'm not yet sure how "reputation scores" or any such thing work here, so don't know if I should mark both your contributions as "correct answer" or what, but you've both been very helpful in me figuring this stuff out :) – Steve Griffiths Sep 15 '12 at 10:37

2 Answers2

1

This may take awhile to answer completely but I'll start with the setPassword question.

your current:

public function setPassword($value) {
        $this->password = md5($value);
    }

Now this has nothing to do with convention or best practice but practicality.

ask yourself:

What happens when you retrieve a database record for your user object and that database record contains a hashed password?

Answer: When you construct the user object and call $this->setPassword($password); or equivalent, you will be applying the hash to a hash.

So you are almost obligated to hash the password in the mapper's save() method or the method used to update the password. Think of the hash value in the database table as the password and the value that's typed into the form field as a placeholder for that password.

Next Part:

To my mind, all the logic pertaining to "fields to do with a user" should either live in its Model, or its Mapper

This is mostly correct.

Everything that belongs to the object domain (Model_User) shall be addressed in the domain Model class (Model_User).

Mappers are only to translate (map) a data object (database row, json string, xml file, flat file, csv file ...) to a form that can instantiate a domain object (Model_User).

So you may end up with more then one mapper available for a given domain object or one mapper may map to more then one source of data.

It might help you if you stopped thinking of your data as "fields", which might tend to keep your head in the database, and instead think of your objects in terms of properties or characteristics.

Because when you get down to the most basic level a Model_User object is just:

class Model_User {
    protected $id;
    protected $name;
    protected $password;
    //continue....
}

all of the getters, setters, constructors and other methods are pretty much so we can put values into those variables.

RockyFord
  • 8,509
  • 1
  • 13
  • 21
  • +1 for the model properties instead of fields. But it's `protected`, not `$protected` right? Anyways, I would put it as private to properly encapsulate and hide the class details. If for somehow I need to extend it, I would use the setters/getters instead of accessing the properties directly. It's related to the *OPEN/CLOSED Principle*. – Keyne Viana Sep 09 '12 at 16:15
  • This looks amazing @RockyFord, thanks so much. Trying to parse it now, but after today's Zend struggles it may take a while... – Steve Griffiths Sep 09 '12 at 16:22
  • @Keyne of course you are correct, that's what I get for writing code without the support of auto complete. :) – RockyFord Sep 10 '12 at 00:40
1

The DataMapper is responsible for populating the object with its data, as well as persisting it. It seems like you're mixing things when you call $user->save() because you're putting persistence logic within your domain object. This is a common approach when you're using the ActiveRecord pattern instead of DataMappers, which is a bad thing.

Your DataMapper should be responsible for saving the object $mapper->save($user); and it needs to update just the changed properties. So, the password will be updated only if you set the new hash.

UPDATE:

You said:

[...] trying to do the md5 step in Mapper_User->save(); method would cause issues if the Model_User were one pulled from the DB, as the password field would clearly already be hashed.

Creates a method called setPasswordHash() and use it when pulling from the database.

Remember: Don't look for things!

Instead of looking for the database inside your mappers, you should ask for it.

public __construct(Zend_Db_Table $dbTable) {
    $this->dbTable = $dbTable;
}

It's all about Dependency Injection.

Keyne Viana
  • 6,094
  • 2
  • 21
  • 53
  • Hi @Keyne thanks for the response. The `save();` method actually is within `Mapper_User`, and a `Model_User` object gets passed in to it, the data extracted, and saved in to the DB via `DbTable_User`. So I think I've got that bit right at least! :) – Steve Griffiths Sep 09 '12 at 16:11
  • @SteveGriffiths Ah! Didn't see that. Sorry =) Also note that you'd be better using `crypt()` with blowfish instead of `md5()` – Keyne Viana Sep 09 '12 at 16:16
  • No worries, and yeah I'll be switching from md5 to something a bit less collision-y once all this other stuff's resolved! Thanks again – Steve Griffiths Sep 09 '12 at 16:21
  • Re: your edit; doesn't that mean that any given Controller wishing to instantiate a given Mapper would also need to be aware of the corresponding DbTable? e.g. inside some Action in some Controller: `$mapper = new Mapper_User(new DbTable_User());`? Seems more complex this way... man this is all so confusing. – Steve Griffiths Sep 09 '12 at 16:34
  • Forgot to tag you in my reply @Keyne – Steve Griffiths Sep 09 '12 at 16:35
  • **Watch that video first.*** Generally a DI (Dependency Injection) component takes care of it (using `reflection`). Also, you should avoid spreading `new Object()` across your application. Object creation is a task of `Factories`. And Yes, it'll be a little complicated at first, but then you'll love it =P Without a DI component, you'd have something like this on your controllers `$userMapper = $mapperFactory->create('UsersMapper');` – Keyne Viana Sep 09 '12 at 16:38
  • Please, follow this link too, it will help in some points: http://stackoverflow.com/questions/5863870/how-should-a-model-be-structured-in-mvc/5864000#5864000 – Keyne Viana Sep 09 '12 at 16:48
  • Watched it, thanks! :) Ok I get the DI thing... I think. First thing I did in Zend was follow a Mapper/DbTable/Model tutorial, then build Abstracts for all three, extended by essentially empty objects, using the $_dependentTables and $_referenceMap from DbTable classes to auto-populate references/dependencies... but I may need to re-look into this. – Steve Griffiths Sep 09 '12 at 17:20
  • Generally I keep the reference map on the database using JOINs and foreign keys (InnoDB) instead of letting ZF performing the queries, because instead of one query ZF creates separated queries for each relation which is much more slow. – Keyne Viana Sep 09 '12 at 17:31
  • Well, the reference map was the first thing I fixed - I have a class `ModelPlaceholder` in to which I stick *where* to find the referenced object (e.g. a Book object would have a ModelPlaceholder for its Author, telling it where to grab it, or conversely Author has a ModelPlaceholder for its Books), and only actually talk to the DB and replace the ModelPlaceholder with a real object if a corresponding `$model_book->getReferenced('Author');` gets called. – Steve Griffiths Sep 09 '12 at 17:38
  • with the intention being that for any lists of referenced things I'd make a separate Model running its own sql query, like you mention, so as to not end up with hundreds of individual DB calls. – Steve Griffiths Sep 09 '12 at 17:39
  • It seems okay. I thought you were referring to [this](http://framework.zend.com/manual/1.12/en/zend.db.table.relationships.html). You might be interested in looking into [LazyLoading/VirtualProxy](http://phpmaster.com/intro-to-virtual-proxies-1/) pattern. You can use it in order to load dependencies on run-time, like the Author's books. – Keyne Viana Sep 09 '12 at 18:08