Going from Spaghetti PHP OOP to SOLID OOP

Question

I am beyond stuck in a half way point between PHP structural and OOP, but I can never achieve proper SOLID patterns despite reading DI, IOC, and reading about the interfaces.

Rather than to read logical examples (i.e. a storage class that takes a book class and you typecast an interface etc...). I want to know how I can take my brain off thinking I got SOLID down but I actually didn't. There is talk about trying to refactor a small chunk but then that small chunk isn't SOLID enough.

Here are the code segments - my thought and goals are after:

class DataID
{
    protected $_id_array;
    protected $_storage;

    public function __construct(StorageInterface $db_conn, $tables = array())
    {
       $this->_storage = $db_conn;
        foreach ($tables as $table) {
            $sql = "SELECT `id` as `id`, `name` as `value` FROM $table ORDER BY `value` ASC";
            $result = $this->_storage->read($sql);
            if (!empty($result)) {
                foreach ($result as $row) {
                    $this->_id_array[$table][$row['value']] = $row['id'];
                }
            }
        }
    }
    public function getID($table, $value)
    {
        if (!empty($this->_id_array[$table])) {
            if (isset($this->_id_array[$value])) {
                return $this->_id_array[$table][$value];
            }
        }

        return null;
    }
}

Goal:

I have tables that has ID vs Value data. I'm going to use this class to run through data (value) and I want to retrieve the ID of it. Best way to avoid too many DB request is to put it inside an array of [value] = ID I believe and then check corresponding index as value and fetch ID.

Storage is an container that goes into the DB via SQL and go fetch me all the data over. This way if the method of the storage changes (Mongo, SQL, pod, MySQLi) I can change how I treat my read.

Tables is a list of known tables in the DB that I need that Data from. That list will be from an array config file that shall be loaded in a main.

Issues:

1. I have a strong feeling that I can make this better but I don't know how - because to me a constructor initialize things and well I'm going into the DB to initialize my array but for some reason i feel that SOLID fails for me here.

2. If I have tables that have too many values (i.e. let's say if a table look up is more than 50) I want to hit the DB and not store it inside an array.

If that's the case, I'll have to make queries inside getID to use storage to go fetch it. How do I maintain SOLID while adding that extra logic?

Answer 1

Reading through this code and your goals, it seems this class has two responsibilities:

• Provide a Key-Value store
• Get data from the database (potentially with caching)

To make it more SOLID, you can pick one of these responsibilities for this class and put the other one somewhere else.

If you wanted to use this class to provide a KV store, you would inject an array pre-filled with all of the data into the constructor rather than injecting DB information. You could then use a factory that takes those parameters to build the DataID object with the data. This removes a dependency on anything database related, simplifies your constructor, and reduces the class to a thin veneer over the underlying array. It cannot, however, get new information by itself, nor can it provide database caching. You could create an adapter class around the database result set and inject that instead of the array. The result set adapter could then handle getting additional information and caching, but this might end up replicating the concerns you have here, just somewhere else.

If you wanted to use this class as an adapter over the StorageInterface class, you would not do all the work in the constructor and simply make calls to the underlying storage object in the getID method. This allows you to simplify the constructor once again, but does not in itself provide caching. You said the StorageInterface implementation provides query-level caching, but you can create a separate adapter wrapper around it that encapsulates your SQL and caches a whole table whenever one element is retrieved from it, and can limit those caches based on table size. Once again you can use a factory method so that your external application does not need to know about this internal adapter. This option seems to fit most closely with what you want to achieve while still having better SOLID-ness.

EDIT:

Here's how this achieves better adherence to the SOLID principles:

• S: This class's responsibilities have been reduced from two to one.
• O: I can add additional functionality by adding decorators and adapters around the various objects to add additional functionality, without having to change those objects directly.
• L: StorageInterface uses Liskov Substitution, but an adapted class around the StorageInterface can also follow Liskov Substitution by adding decorators around it to add functionality.
• I: The adapters are small, client-focused APIs.
• D: StorageInterface is an abstraction, and your StorageInterface adapters/decorators can also be build as abstractions.

Bonus Points:

By removing the work out of the constructor, you are adhering more closely to the Law of Demeter. Rather than asking for something to get something through it, you are asking for what you really want.