Tag: Learn from my mistakes

I’m writing a web service for conferences. I’ve been writing it, on and off for 3 years, and I think it would be fair to say the coding reflects my learning over those three years. The script is written in PHP. It has a MySQL backend. It has undergone a lot of changes.

In the beginning, was a concept. The concept was to have a digital timetable. One where you could register your interest in a talk, and the busiest talks got the biggest rooms. It should use any tech the user had to use the system including SMS. It would not expect you to have a user name and password combo (heaven forbid!), but would use OpenID.

The concept was implemented and demo’d at an event 2 years ago, and a friend [1] asked where the API was. “API?” I replied, feeling somewhat foolish, “It doesn’t have an API“.

[1] Lorna Jane Bowman (neé Mitchell aka @lornajane)

I realised the foolishness of my coding when discussing this issue [2]. “It’s dead simple, just create another view which returns just JSON” said someone. “View? I don’t know what you mean“… “Model, View, Controller? MVC – it’s a pretty common pattern”. “Oh no” I replied, “my project has inline PHP. It just seemed simpler that way.” “Well, how about you add a toString() function to your classes, and use that to render the output?” “Classes. Another thing my project doesn’t have. Sorry. Can’t do that either.”

[2] With Lorna again, and adding Katherine Reeve (aka @BinaryKitten)

Did you ever get that slightly sinking feeling that maybe you’re making a fool of yourself?

“Well, there are lots of benefits to Classes in PHP, not least of which that you can use PHP CodeSniffer to enforce your coding standards” I start to laugh a little at this point “and you can use PHP Documenter to create all your developer documentation” which shut me right back up again “you can use PDO to create your objects from your database results” wow… mind blown – I’ve jumped on this little trick of a pony… “and of course, you can’t really do unit testing without classes”. Uh oh. “What’s unit testing?” “It’s a set of automated tests you can run against your code every time you commit to your version control software” whew! I’m using that at least! “to make sure that the code you’re committing hasn’t broken anything”.

Fast forward to this week, and I asked on Facebook whether anyone could teach me how to do Unit Testing. See, I’ve managed to cobble together my own MVC – I have classes called “Object_” which are the Models, the controller is my routing script – otherwise known as index.php, and then the Views are templates in Smarty, or just a json_encode((array) $object) [3] for my API. I have my own set of tests – not unit tests, it’s a script called “Test.sh” which runs PHP against the file (to make sure that the pages don’t have brackets missing or similar), then runs PHP Code Sniffer against it, and finally, if all the files are OK, it then runs PHPDoc against the whole mass of it.

[3] As the Apple iPhone/iPad adverts say – some sequences shortened

So, one of the books suggested to me, again by the lovely Lorna Jane, was The Grumpy Programmer’s Guide To Building Testable PHP Applications which mentioned that it’s much easier to do unit testing if you set up dependency injection. Note, I’m still not yet doing Unit testing here.

Woah. What the hell is Dependency Injection? Well, fortunately, there were code examples in that book. Oh boy, were there code examples. So let’s look through this idea. Instead of writing

$stuff = new MyStuff();

class MyStuff()
{
    function construct()
    {
        $this->db = mysql_connect('localhost', 'username', 'password');
        mysql_use_database('production', $this->db);
    }
}

You could instead write this:

$db = mysql_connect('localhost', 'username', 'password');
mysql_use_database('production', $this->db);
$stuff = new MyStuff($db);

class MyStuff()
{
    function construct($db = null)
    {
        $this->db = $db;
    }
}

So, this now means that in your testing framework, you could pass it a non-production database, or a testing database, or, well, anything that relies on something outside your script.

I did a bit of digging around to find some other examples of dependency injection code that might be a bit easier to use, which is to say, something so I don’t need to amend all my constructor functions.

I found this slideshare from a talk at PHP Barcelona about dependency injection which says you can do dependency injection like this:

$thing = new MyClass($dependency);

OR

$thing = new MyClass();
 $thing->setDependency($dependency);

OR

$thing = new MyClass();
 $thing->dependency = $dependency;

but somewhat weirdly, it also says that you can create a container class which holds your dependencies, and refer to that later – and that this isn’t a singleton. Sadly, I didn’t understand all that code fully (and have gone on to file a bug for the PHP documentation for the functions I didn’t understand to help people who follow behind with it!), but (and I’ve copied this verbatim from the slideshare) essentially, it looks like this:

class Container { protected $values = array(); function __set($id,$value) {  $this->values[$id] = $value;  }  function __get($id) {  if (!isset($this->values[$id])) {  throw new InvalidArgumentException(sprintf('Value "%s" is not defined.', $id));  }  if (is_callable($this->values[$id])) {  return $this->values[$id]($this);  } else { return $this->values[$id];  }  }  function asShared ($callable) {  return function($c) use ($callable) {  static $object; if (is_null($object)) {  $object=$callable($c);  }  return $object; }; } } $container = new Container(); $container->session_name='SESSION_ID'; $container->storage_class='SessionStorage'; $container->user = $container->asShared( function($c) {  return new User($c->storage);  } ); $container->storage = $container->asShared(  function($c) {  return new $c->storage_class($c->session_name); } );

Now, I have to be honest, this confuses the hell out of me. How on earth do I use this in my code? I’ve been doing this in my code thus far:

class Object_User{
  protected $intUserID    = null; // Obtained by the getCurrentUser() function
  protected $strUsername  = null;
  protected $hashPassword = null; // More of these, depending on the SQL

  function getCurrentUser() { // Called as $user = Base_User::getCurrentUser();
 $objCache  = Base_Cache::getHandler(); // A singleton to "cache" any data we've pulled to save getting it repeatedly
    if (
          isset($objCache->arrCache['Object_User']['current'])
          && $objCache->arrCache['Object_User']['current'] != false
        ) {
      return $objCache->arrCache['Object_User']['current'];
    }
    $arrRequest  = Base_Request::getRequest(); // Returns array of parsed request data
    $objDatabase = Base_Database::getConnection(); // Returns a PDO object

    $sql = "SELECT * FROM users WHERE strUsername = ? and hashPassword = ?";
    $query = $db->prepare($sql);
    $query->execute(array($request['username'], $request['password'])); $result = $query->fetchObject('Object_User');
    if ($result != false) {
      $objCache->arrCache['Object_User']['id'][$result->getVal('intUserID')] = $result;
      $objCache->arrCache['Object_User']['current'] = $result;
    }
    return $result;
  }

  function getVal($key) { // Return protected variables
    if (!isset($this->$key)) {
      return false;
    }
    return $this->$key;
  }
}

I know that singleton methods are considered “Bad” because they’re (apparently) difficult to unit test, but I would have thought that it would have been pretty straightforward to create a singleton class which holds all the dependencies (see following)

class Base_Dependencies
{
  protected static $handler = null; protected $arrDependencies = array();   protected function GetHandler() {
    if (self::$handler == null) { self::$handler = new self(); }
    return self::$handler;
  }

  function set($key, $dependency) {
    $handler = self::GetHandler();
    $handler->arrDependencies[$key] = $dependency;
  }

  function get($key) {
    $handler = self::GetHandler();
    if (isset($handler->arrDependencies[$key])) {
      return $handler->arrDependencies[$key];
    } else {
      return false;
    }
  }

  function unset($key) { // Only used for Unit Testing, I would imagine
    $handler = self::GetHandler();
    if (isset($handler->arrDependencies[$key])) {
      unset($handler->arrDependencies[$key]);
    }
  }
}

Doing it this way means I can, from, for example, my database class, which is currently a singleton, say instead:

function GetConnection() {
  $db = Base_Dependencies::get("Database");
  if ($db != false) {
    return $db;
  }
  $config = Base_Config::getAllConfig();
  $db = new PDO($config['DSN'], $config['DB_User'], $config['DB_Pass']);
  Base_Dependencies::set("Database", $db);
  return $db;
}

Is this wrong? Is this just not best practice? Given the above, how can I fix my dependencies in such a way that the poor schmuck who wants to commit a patch can figure out what they’re sending? Or do I just need to fix how my head works with this stuff? If it’s the latter, can someone provide some samples of what I’m doing wrong?

Thanks for reading this mammoth post!