"Property Container" design-pattern

Question

I've tried to write my Property Container design-pattern implementation.

Could anybody, please, tell me, if this code is really what I intended to write (follows the Property Container design-pattern rules)? Is there anything that can be improved?

<?php

class PropertyContainer
{
    private $PropertyContainer = array();

    public function __construct() { }

    public function addProperty($k, $v)
    {
        for($i = 0; $i < count($this->PropertyContainer); $i++) { }
        $this->PropertyContainer[$k] = $v;
    }

    public function setProperty($k, $v)
    {
        while($this->PropertyContainer)
        {
            if(key($this->PropertyContainer) == $k) {
                $this->PropertyContainer[$k] = $v;
                return;
            }
            next($this->PropertyContainer);
        }
        echo "Key was not found";
    }

    public function getProperty($k)
    {
        //var_dump($this->PropertyContainer);
        foreach($this->PropertyContainer as $key => $val)
        {
            if($key == $k) { return $val; }
        }
        echo "Key was not found";
        return;
    }
}

$pc1 = new PropertyContainer();
$pc1->addProperty("myProperty1", 31);
$pc1->addProperty("myProperty2", 32);
$pc1->addProperty("myProperty3", 33);
$pc1->setProperty("myProperty2", 7);
echo $pc1->getProperty("myProperty1") . "<br />";
echo $pc1->getProperty("myProperty2") . "<br />";
echo $pc1->getProperty("myProperty3") . "<br />";

echo "<br />";

$pc2 = new PropertyContainer();
$pc2->addProperty("myProp1", 11);
$pc2->addProperty("myProp2", 11);
$pc2->addProperty("myProperty3", "Some String");
$pc2->setProperty("myProp2", 12);
echo $pc2->getProperty("myProp1") . "<br />";
echo $pc2->getProperty("myProp2") . "<br />";
echo $pc2->getProperty("myProperty3") . "<br />";

echo "<br />" . $pc2->getProperty("myProperty5") . "<br />";

?>

Answer 1

Yes, you are using this patter correctly here. Its use in PHP (especially as a generic implementation) is greatly reduced though. As the answers in your programmers.stackexchange.com point out: basically it is a hashmap. Now, PHP implements arrays as some form of hash map already. I see this pattern as an anti-pattern too. There are some use-cases of this pattern when you register validators along each property this property has to fullfil when setting.

The php class stdClass basically does what you want to achieve it a more direct way:

$object = new stdClass();
$object->key = $value;

Though I'd recommend not to use this code in production, it is a good way to learn of course.

So my review for your code:

• $PropertyContainer: It is uncommon in PHP to have variables start in upper case. Most code I do read either starts in lower-case and follow Camel-Case or is snake casing.
• public function __construct() { } just fills up space. Not required, therefore remove it.
• You should have a closer look at how to work with arrays in PHP. Your current approach is highly inefficient. Many of the array functions do not relay on the array iterator's state. Currently you force PHP to iterate over the array. If you use dedicated methods instead, PHP doesn't have to iterate (which is much faster of course :))
  • To add an entry to your array there is no need to iterate over it without doing it first. You can either array_push it at the end or just set it by $this->PropertyContainer[$k] = $v.
  • Same goes for checking of existence. Use isset instead.
  • And for accessing you can use $this->PropertyContainer[$k] instead of iterating
• $k and $v are bad parameter names. While they usually are recognized as key and value, there is no harm to call them this way. But this can be improved: you are talking about properties and not keys. So better name them $propertyName and $value. Shorthands should be used in local scope only (if at all).
• While I suppose they are just there for debugging: echo'ing on error is a bad idea. Either return a error code or even better, throw an exception. Of course this requires a method to check if a property exists (e.g. hasProperty).
• return; without any value is the default anyway at the end of a method. You can remove this, shorting the code. Elsish after some time another developer might wonder why it was written once: was it located at some other line? was there some return value previously?