views:

64

answers:

2

I recently came across an article by Matthew Weier O'Phinney (ZF project lead) that contains sample code similar to this:

class User
{
    protected $_data = array(
        'username' => null,
        'email'    => null,
        'fullname' => '',
        'role'     => 'guest',
    );

    /* ... */
}

Notice how what would traditionally be four different member variables is consolidated into one array. I can see the benefit of the clean constructor (only one parameter), but I doubt IDEs will be able to do auto-completion very well on the $_data array.

One alternative I can think of is to use magic methods to have a constructor with a single parameter and the four members.

class User
{
    protected $_username = null;
    protected $_email = null;
    protected $_fullname = '';
    protected $_role = 'guest';

    public function __construct($data)
    {
        foreach ($data as $key => $value) {
            $this->$key = $value;
        }
    }

    public function __set($name, $value) {
        $member = "_$name";
        $this->$member = $value;
    }
}

The second block of code seems better...however I doubt I can write better code than Mr. O'Phinney. What is the best way to handle class members while still keeping a clean interface to the constructor?

+2  A: 

The big purpose behind the everything-in-an-array method as shown in the article is so that the __set method can control what gets set and what doesn't by checking the array first. Your current __set doesn't do that, but it's a simple addition:

public function __set($name, $value) {
    $member = "_$name";
    if(property_exists($member, $this)) // <- New magic.
        $this->$member = $value;
}

You'd want to do the same thing in your constructor.

However, you're still going to have autocomplete issues. Modern IDEs that know enough to show instance methods and variables are also frequently smart enough to show or hide things in the list based on whether they're public, protected or private. Declaring everything protected will basically have the same effect as hiding them in the array, as far as the IDE is concerned. This is a major downside to using protected instance variables and __set.

Charles
+1  A: 

It's all a trade-off. MO'P's code does indeed cause problems if you like to rely on your IDE's autocompletion. The upside is that you have a nice handy internal array that can make things like implementing a __toArray() function pretty easy.

The compromise you suggest seems silly to me. Your IDE still won't be able to help you when constructing arguments, and your magic __set() basically just makes your protected properties effectively public. You could add magic just like in Charles' answer, with the same caveats that mentions (namely, that you'll still have autocomplete issues).

I've also learned a lot from MO'P's posts, but in general it's not wise to implement things just like he does. Most of his example code is there to demonstrate a concept, and not demonstrate a practical real-world approach.

Single-argument constructors can be very useful if you have a lot of optional configuration keys. In a perfect world, perhaps there would be some extension to phpDoc so you could document valid configuration array keys for constructors.

That said, in the meantime, if you want to rely on autocomplete, and don't want an explicit set of pseudoproperties for things like __toArray(), you're better off just creating public properties or set-methods, adding phpDocs, and not having the constructor set any of them. Just force consuming code to explicitly configure the object by assignment or method calls.

timdev
Makes good sense, thanks.
Joshua Johnson