Hey guys,
I have been thinking a lot about this. I’ve seen this case too many times when a parent class extended by a child class holds methods common to say 2 out 10 controllers. But in the name of don’t repeat yourself this is justified. I think its just easy to get away with this sort of code. But if you understand how incredibly powerful and useful polymorphism is in making your code stable and easy to modify i.e the reason why you should extend you classes you would not do this again.
classic example
class DirectDebitPaymentController extends Controller { public function indexAction() { ... $this->setTemplate('DirectDebitPayment'); $scheme = $this->getDirectDebitPaymentScheme($_GET['Scheme']); $this->setViewParam('scheme', $scheme); $this->setViewParam('param1', $param1); $this->setViewParam('param2', $param2); return $this->template; } }
Simples, no probs. Will do. Then comes along another requirement to have another screen that will do the same, but in a different layout called a “Scheme”. The usual, you decide to pass in a param to say its the “Scheme” case. Your code becomes:
class DirectDebitPaymentController extends Controller { public function indexAction() { ... $this->setTemplate('DirectDebitPayment'); if (! empty($_GET['Scheme'])) { $userData = $this->getUserData($_GET['UserId']); $scheme = $this->getDirectDebitPaymentScheme($userData['schemeName']); } else { $scheme = $this->getDirectDebitPayment($_GET['paymentId']); } $this->setViewParam('scheme', $scheme); $this->setViewParam('param1', $param1); $this->setViewParam('param2', $param2); return $this->template; } }
Yea that will work. But consider what you’ve done. You’ve potentially introduced bugs in the first and the second iteration! On top of that you’ve messed up perfectly good visibility around your code. And this is just the beginning of things. The stuff that would happen in the view is even worse than this probably.
Another way to do this would be to create another controller, yeah that makes sense.
class DirectDebitSchemePaymentController extends Controller { public function indexAction() { ... $this->setTemplate('DirectDebitPayment'); $userData = $this->getUserData($_GET['UserId']); $scheme = $this->getDirectDebitPaymentScheme($userData['schemeName']); $this->setViewParam('scheme', $scheme); $this->setViewParam('param1', $param1); $this->setViewParam('param2', $param2); return $this->template; } }
This is better and by far much cleaner. But hey – we’ve just duplicated code. Chances are, as time goes by, both these versions will use code that is similar since they are just different versions of the same functionality. So how do we use the functionality from the base version and create another version? Polymorphism. Just extract out pieces of code that you’d like to override in your controller.
class DirectDebitPaymentController extends Controller { public function indexAction() { ... $this->setTemplate('DirectDebitPayment'); $scheme = $this->getUserScheme($_GET); $this->setViewParam('scheme', $scheme'); $this->setViewParam('param1', $param1); $this->setViewParam('param2', $param2); return $this->template; } protected function getUserScheme(array $data) { if (! isset($data['paymentId'])) { throw new Exception('paymentId required for call'); } return $this->getDirectDebitPayment($data['paymentId']); } }
And in your child class, extend this controller
class DirectDebitSchemePaymentController extends DirectDebitPaymentController { protected function getUserScheme(array $data) { if (! isset($data['UserId'])) { throw new Exception('UserId required for call'); } $userData = $this->getUserData($data['UserId']); return $this->getDirectDebitPaymentScheme($userData['schemeName']); } }
That is it. Done. Your controller handler will call upon indexAction() method which exists in the DirectDebitPaymentController class. That action method will call upon the highest available member of getUserScheme(), which is the one we’ve just overridden.
You may be thinking that “Yo! you just modified the original so what difference does it make anyway?” Yeah that is true. You cannot foresee the future, so a modular approach is always best to begin with i.e create lots of private methods and open them up gradually. Extracting out the same functionality in a method is far less destructive than if statements. If you create multiple paths within the same controller i.e multiple if statements – you would have to test out all the paths to say that it doesn’t break the original version. By doing it this way, you test that version in isolation of the other. And you can go on creating lots of other versions which won’t affect the other. There is really good separation here.
Almost all of the major coding principles talk about polymorphism. The best I know is SOLID principles. Specifically the “OLI” can be achieved with good polymorphism.
O = Open/Closed principle
L = Liskov Substitution Principle
I = Interface segregation principle
Strange terms above right? So what do these mean. The “Open/Closed principle” states that a piece of code should be open for extension but closed for modification. What does that mean!?
Simply put, if you have a base class that is utilised by other classes for instance, the code should not change in functionality, but different flavours of it could be created by extending it. This is where Polymorphism kicks in. Your class should be modularised so any method no matter how complex it is, parts of it can be overridden by a child class. This is how you reduce code duplication and produce stable code. Moving common code to a parent class is just masking the issue. Your parent should contain methods that actually belong in it and lie within its responsibilities, probably not because its shared between 2 out of 10 controllers.
This solution then expands itself into the L “Liskov Substitution Principle” that states “objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.” Instances that are truly a subtype of their parent are easily substitutable. Preferably with interfaces which is also one of the most mis-understood concept of our time.
This constitutes the “I” of the SOLID principles which states “many client-specific interfaces are better than one general-purpose interface.” Create more interfaces where possible and extend them as you would a class where possible. It is also important to understand that when an application consumes an external package, your application should define its requirements, not the package. i.e your main codebase should not implement a package interface. In fact you should create another interface that extends the package interface, and type hint it with that interface. This ensures that if you are to replace a package – you don’t have to change every reference of that interface – just one.
If you’ve got common functionality that is re-usable, create another class that is responsible for representing that functionality, and inject that class as a dependency. With frameworks in place these days this seems like a difficult thing to do, but you know what? Its just another class, autoloaded in like the rest of them. Symfony uses services to do this, yeah that is a fancy name for just another class, but represents a class which has an application wide scope.
Did you know we just covered the “D” and “S” of the SOLID principles? Yeah that simple.
D = Dependency inversion principle “one should depend upon abstractions, [not] concretions.”
S = Single responsibility principle “a class should have only a single responsibility”
Next time you are about to move code into the parent class because its the quickest and easiest thing you can get away with, think about it. That piece of code may be a lot more useful in its own class being re-used throughout your app/fully unit tested. As for your parent – the lighter it is, the easier it is to maintain.
This is SOLID principles in some limited capacity. Their is a lot more to it, but this forms a good starting point.
Thoughts?
Michal
Hi there! Such a wonderful article, thanks!
www.reverbnation.com
Hello, Neat post. There’s a problem with your website in web explorer, might test this?
IE still is the marketplace chief and a big section of
people will omit your excellent writing due to this
problem.