4
\$\begingroup\$

I've put together a simple application in order for me to learn sqlite and OOP in PHP. The application basically outputs two random words, e.g. "Angry Badger", from two database tables, one called Animals & the other named Adjectives. At present both tables contain the same columns (but could change in future).

The code uses an interface, iWord with two methods to be implemented by their concrete classes (Animal, Adjective). The database object has been passed in on the constructors so I can create some unit tests later.

If I were to introduce a new class, fruit, with the same methods: getRandomItem() & listRandomItems($numerOfItems) I would need to create the another DB table, class, implement the same DB error handling and processing of data. This doesn't feel correct.

An alternative may be to combine the database tables into one and have one class that I pass in an item to lookup, e.g. animal, fruit, adjective, & dynamically build the SQL.

Any recommendations or patterns that I should be following?

Please note: this is test code for learning & not for production use ;)

<?php

interface iWord
{
    public function getRandomItem();
    public function listRandomItems($numerOfItems);
}

class Animal implements iWord
{
    private $database;

    public function _construct(PDO $pdo)
    {
        $this->database = $pdo;
    }

    public function getRandomItem() 
    {
        $rAnimal = NULL;
        try 
        {
            $query = $this->database->prepare("SELECT animalName FROM animal ORDER BY RANDOM() LIMIT 1;");
            $query->execute();
            $sqlres1 = $query->fetch(PDO::FETCH_OBJ);
            $rAnimal = $sqlres1->animalName;
        }
        catch (PDOException $ex)
        {
            echo $ex->getMessage();
        }
        return $rAnimal;    
    }

    public function listRandomItems($numerOfItems)
    {
        // not yet implemented
        throw new Exception('Not Implemented');
    }
}

class Adjective implements iWord
{
    private $database;

    public function _construct(PDO $pdo)
    {
        $this->database = $pdo;
    }

    public function getRandomItem() 
    {    
         // exception handling required
         $query = $this->database->prepare("SELECT adjective FROM adjectives ORDER BY RANDOM() LIMIT 1;");
         $query->execute();
         $sqlres2 = $query->fetch(PDO::FETCH_OBJ);
         return $sqlres2->adjective;
     }

    public function listRandomItems($numerOfItems)
    {
        throw new Exception('Not Implemented');
    }    
}

try {
    $pdo = new PDO("sqlite:c:/htdocs/mydb.sqlite");
    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch(PDOException $e) {
    echo $e->getMessage();
}

$adj = new Adjective($pdo);
$rAdjective = $adj->getRandomItem();
$anm = new Animal($pdo);
$rAnimal = $anm->getRandomItem();

echo $rAdjective . " " . $rAnimal;

unset($pdo); 
\$\endgroup\$
1
  • \$\begingroup\$ Just a quick suggestion: you can use Registry pattern for db connections in order to avoid passing it to the __construct(). Here is an example: bitbucket.org/t1gor/strategy/src/… \$\endgroup\$
    – t1gor
    Apr 11, 2014 at 13:11

2 Answers 2

1
\$\begingroup\$

Extract an abstract base class AbstractWordTable that implements the better-named interface WordTable by moving all code that's duplicated between Adjective and Animal (all but the table and column names). This is a major step in the DRY direction that will simplify your code.

I won't attempt to produce this code on my phone, but the key is for those concrete implementations to contain only unique code. The end result will be with a simple one-line constructor.

Another improvement would be to decouple reading the table contents from picking random words by splitting these functions between WordTable that reads words from a database table into an array and WordList that provides various ways to consume it, including choosing random words and subsets.

\$\endgroup\$
1
\$\begingroup\$

This code is probably challenging because it is dealing with two different levels of abstraction: getting a word, and talking to a database. We need to separate those concerns into abstractions. This also brings us up against the Law of Demeter, as we are asking for a database object, only to reach through it and get something else. Since you want to talk patters, I'll talk about two: Iterator and Template.

I'd first replace the database parameter with an iterator, namely a RandomWordIterator (which will be an interface). The interface will look as so:

interface RandomWordIterator {
    public function next();
}

You can also add on other typical iterator methods like hasNext(), but since it looks like it will represent a random, infinite stream, this should suffice. Now your classes become like:

class Animal implements iWord
{
    private $wordIterator;

    public function _construct(RandomWordIterator $wordIterator)
    {
        $this->wordIterator = $wordIterator;
    }

    public function getRandomItem() 
    {
        return $this->wordIterator->next();    
    }

    public function listRandomItems($numerOfItems)
    {
        $arr = array();
        for($i = 0; $i < $numberOfItems; $i++)
        {
            $arr[] = $this->getRandomItem();
        }
        return $arr;
    }
}

Now you'll need to implement a RandomWordIterator. We could build one based around an array, or a web service, or a database. This flexibility could be useful in production, and is definitely useful in testing. Let's create an abstract (I'll explain why abstract soon) DBTableRandomWordIterator to start off.

abstract class DBTableRandomWordIterator implements RandomWordIterator
{
    protected $database;

    public function __construct(PDO $pdo)
    {
        $this->database = $pdo;
    }

    abstract function getTableName();
    abstract function getColumnName();

    public function next()
    {
        $tableName = $this->getTableName();
        $columnName = $this->getColumnName();
        $rWord = NULL;
        try
        {
            $query = $this->database->prepare("SELECT $columnName AS word FROM $tableName ORDER BY RANDOM() LIMIT 1;");
            $query->execute();
            $sqlres1 = $query->fetch(PDO::FETCH_OBJ);
            $rWord = $sqlres1->word;
         }
         catch(PDOException $e)
         {
             echo $ex->getMessage();
         }
         return $rWord;
    }
}

We've created a base implementation, while leaving open hooks for specifying details through subclassing (thus the abstract). Now we can make use of the second pattern, the Template pattern (though it could just as easily be called the Factory pattern as you are making string objects, but in this case the result is the same). We now implement concrete classes for each table type, implementing the two abstract methods.

class RandomAnimalIterator extends DBTableRandomWordIterator
{
    public function getTableName() { return 'animal'; }
    public function getColumnName() { return 'animalName'; }
}

We need one concrete class per table. This localizes changes when the database tables change, and allows us to vary table and column name formats with ease.

Now let's put all the pieces together:

try {
    $pdo = new PDO("sqlite:c:/htdocs/mydb.sqlite");
    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch(PDOException $e) {
    echo $e->getMessage();
}

$adj = new Adjective(new RandomAdjectiveIterator($pdo));
$rAdjective = $adj->getRandomItem();
$anm = new Animal(new RandomAnimalIterator($pdo));
$rAnimal = $anm->getRandomItem();

echo $rAdjective . " " . $rAnimal;

unset($pdo);

Now, this is a complete solution using patterns, but it may bother people that classes like Adjective and Animal are basically empty shells now. I would probably combine those classes with their respective iterators if I were making this as a real project, though there is no requirement to do so. Leaving them separate may allow for separation of responsibilities at some point later if you start to hang more behavior off of those classes like Animal and Adjective.

The other option could have been to have a single, concrete DBTableRandomWordIterator and pass in a delegate object containing the table and column names. This would change the relationships a bit, and could allow for better object reuse (a single DBTableRandomWordIterator where the delegates are swapped out), with all the caveats expected.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.