If you look at the source code of CakePHP you find sometimes a piece of code where you have to say to yourself: “Wow, that’s a cool example of a bad practice! That could be used to teach principle x!” (of course, you will find such examples in other projects, too)

I found one such example as I used the core class ClassRegistry in a scenario which could be simplified as shown below:

class A {
    function id() {
        return 'A';
    }
}

class B {
    function id() {
        return 'B';
    }
}

$a = new A();
$b = new B();

ClassRegistry::addObject('test', $a);
ClassRegistry::addObject('test', $b);

$class = ClassRegistry::getObject('test');
echo $class->id();

What will be the output of this example? As you may guess the output is not “B” but “A”. Not really what you would expect. If we look at the code of the addObject method (rev. 5671) we see the reason for this result:

/**
 * Add $object to the registry, associating it with the name $key.
 *
 * @param string $key	Key for the object in registry
 * @param mixed $object	Object to store
 * @access public
 */
function addObject($key, &$object) {
    $_this =& ClassRegistry::getInstance();
    $key = Inflector::underscore($key);
    if (array_key_exists($key, $_this->__objects) === false) {
        $_this->__objects[$key] = &$object;
    }
}

An object gets only added to the registry if the respective key is not already used. If the key already exists, nothing happens and no error is thrown. So, to be sure your object gets added, you have to call removeObject() first… I don’t know why it was implemented in this way and at the moment I can’t imagine a reason why it was realized like that.

Ok, what could be done better?

The easiest and, in my opinion, the best solution is to implement it in the same way other structures to store key/value pairs work, i.e. an object with a certain key gets overwritten if a new object is added with the same key. To get this functionality, only the check for the key existence has to be removed.

If you want to keep the functionality as it is implemented now, then I think you have to do two changes: modify the API comment so that it describes what the method really does, and throw an error if someone tries to add an object with a key that already exists.

The main lesson for me from this example is: Don’t implement a known concept differently, unless you have a very good reason to do so.