Don't be STUPID: GRASP SOLID!

Ever heard of SOLID code? Probably: It is a term describing a collection of design principles<br>for “good code” that was coined by Robert C. Martin (aka “uncle bob”), our beloved evangelist<br>of clean code.

Programming is full of acronyms like this. Other examples are DRY (Don’t Repeat Yourself!) and<br>KISS (Keep It Simple, Stupid!). But there obviously are many, many more…

So, why not approach the problem from the other side for once? Looking at what makes up bad<br>code.

Sorry, but your code is STUPID!

Nobody likes to hear that their code is stupid. It is offending. Don’t say it. But honestly: Most<br>of the code being written around the globe is an unmaintainable, unreusable mess.

What characterizes such code? What makes code STUPID ?

S ingleton

T ight coupling

U ntestability

P remature Optimization

I ndescriptive Naming

D uplication

Do you agree with this list? Yes? Great. No? I’ll explain the individual points in greater detail<br>in the following, so you can better understand why exactly those patterns were chosen.

Singleton

class DB {<br>private static $instance;

public static function getInstance() {<br>if (!isset(self::$instance)) {<br>self::$instance = new self;

return self::$instance;

final private function __construct() { /* something */ }<br>final private function __clone() { }

/* actual methods here */

The above is the typical database access implementation you will find in pretty much any PHP<br>tutorial. I actually used something similar to this myself not too long ago.

Now you wonder: What’s wrong with that? You can easily access the DB from anywhere using<br>DB::getInstance() and the code also ensures that you only have one database connection open at a<br>time. What could be bad about that?

Well, yeah, I thought that too ^^ “I only need one connection.” When the application grew larger it<br>turned out that I actually needed a second connection to a different database. And that’s where the<br>mess began. I changed the singleton to have a ->getSecondInstance(), thus converting the singleton<br>to a .. erm .. dupleton? Instead I should have realized that the database connection simply isn’t a<br>singleton and thus can’t be sanely implemented as one. The same also applies to any other use of<br>singletons that you will find. The request object surely is a singleton! Ever heard of subrequests?<br>But the logger definitely is! Never wanted to log different things differently?

And that was only one issue. Another important issue is that by using DB::getInstance() in your<br>code you are binding your code to the DB classname. This means: You can’t extend the DB class.<br>At some point I wanted to optionally log query performance data to APC. But the tight coupling to<br>the class name did not allow me to. If my application had used dependency injection at that<br>time I could have easily extended DB and passed the new instance. But the singleton prevented.<br>What I did instead was somethink looking roughly like this:

// original DB class<br>class _DB { /* ... */ }

// extending class<br>class DB extends _DB { /* ... */ }

One word: Ugly. One could add some other words like Hacky, Unmaintainable, Crap. Or STUPID.

One last point to consider: Remember when I said “You can easily access the DB from anywhere using<br>DB::getInstance()”. Well, actually that’s a bad thing too. Read “from anywhere” as “globally” and<br>translate to “A singleton is a global variable with a fancy name.” When you learned PHP you were<br>probably told that it’s evil to use the global keyword. But by using a singleton you are doing<br>just that: creating global state. This creates non-obvious dependencies and thus makes your app hard<br>to reuse and test.

Tight coupling

You can actually generalize the Singleton issue to static methods and properties in general.<br>Whenever you are writing Foo::bar() in your code you are tightly coupling your code to the Foo<br>class. This makes extending Foos functionality impossible and thus makes your code hard to reuse<br>and hard to test.

Similarly most other plain uses of class names are code smell too. This also applies to the new<br>operator:

class House {<br>public function __construct() {<br>$this->door = new Door;<br>$this->window = new Window;

How would you replace the door or the window in your house now? Simple: You can’t. As a good<br>developer you will obviously find some dirty hack that does allow you to somehow replace the<br>door or a window. But why not simply write this instead:

class House {<br>public function __construct(Door $door, Window $window) { // Door, Window are interfaces<br>$this->door = $door;<br>$this->window = $window;

This way one can easily create houses with different doors and windows. The code is easy to extend,<br>easy to reuse and easy to test. What could you want more?

The above code is actually “dependency injection” in a nutshell. Many people associate DI with<br>some Dependency Injection Container (DIC) like Symfony’s, but the actual concept of DI is so much<br>simpler.

Untestability

Unit testing is important....