By Benjamin Eberlei, first published at Tue, 16 May 2017 10:25:18 +0200
Download our free e-book "Crafting Quality Software" with a selection of the finest blog posts as PDF or EPub.
You can also buy a printed version of the book on Amazon or on epubli.
Software systems usually get more complex over time. In the beginning a variable starts out to represent something very simple with very few rules and constraints that can are enforced in a single location of the code.
Take this code example where the user selects a start and an end date to query a list of events:
class EventController
{
public function listAction(Request $request)
{
$start = new \DateTime($request->query->get('start', '-60 minute'));
$end = new \DateTime($request->query->get('end', 'now'));
if ($start > $end) {
$tmp = $end;
$end = $start;
$start = $tmp;
}
return [
'events' => $this->eventRepository->findBetween($start, $end),
];
}
public function listTodayAction()
{
$start = new DateTime('today 00:00:00');
$end = new DateTime('today 23:59:59');
return [
'events' => $this->eventRepository->findBetween($start, $end),
];
}
}This simple switch of start and end date when they are inverted is common and the simplicity of the code often means it is copied rather than abstracted into a method.
But how do we extract a method for this code? We could add a method switchStartEnd() on the EventController, but look how ugly that looks like:
private function switchStartEnd($start, $end)
{
if ($start > $end) {
$tmp = $end;
$end = $start;
$start = $tmp;
}
return array($start, $end);
}
public function listAction(Request $request)
{
$start = new \DateTime($request->query->get('start', '-60 minute'));
$end = new \DateTime($request->query->get('end', 'now'));
list ($start, $end) = $this->switchStartEnd($start, $end);
return [
'events' => $this->eventRepository->findBetween($start, $end),
];
}Plus, the biggest downside of this refactoring is the fact that you cannot use switchStartEnd in other places that perform date range handling.
The problem here is a code smell that is widespread in every codebase I have ever seen and is called "Primitive Obsession". It means that as developers we often rely on the most basic types of our programming language, instead of increasing the abstraction and introducing new types. In object oriented programming a type is equivalent to a new class.
Object oriented systems often have tons of classes that work on fullfilling a use-case, but they are not really types like string, integer or DateTime are.
In our example we are missing a DateRange class, and introducing it will immediately simplify our code and allow us to heavily unit-test business logic related to date ranges.
class DateRange
{
public function __construct(DateTime $start, DateTime $end)
{
if ($start > $end) {
$tmp = $end;
$end = $start;
$start = $tmp;
}
$this->start = $start;
$this->end = $end;
}
public function getStart()
{
return $this->start;
}
public function getEnd()
{
return $this->end;
}
}Writing a unit-test for this is simple. Writing a test for the same code embedded into the Controller may be way too much work for the benefit.
We don't have to stop here though, we also have code constructing the DateRange in our controller that we can extract into the new value object:
class DateRange
{
public static function today()
{
$start = new DateTime('today 00:00:00');
$end = new DateTime('today 23:59:59');
return new self($start, $end);
}
public static function fromStrings($start, $end)
{
return new self(new DateTime($start), new DateTime($end));
}
}Again, these methods on the DateRange can be easily tested. If we use the DateRange everywhere in our code we could easily add more code into the fromStrings method that does proper error handling when the strings are not valid dates for example.
Meanwhile the controller code is refactored into something very boring, all the logic is hidden in small testable classes:
public function listAction(Request $request)
{
$range = DateRange::fromStrings(
$request->query->get('start', '-60 minute'),
$request->query->get('end', 'now')
);
return [
'events' => $this->eventRepository->findBetween($range),
];
}
public function listTodayAction()
{
return [
'events' => $this->eventRepository->findBetween(DateRange::today()),
];
}Introducing value objects is extremely helpful in structuring data and making small business rules reusable and abstracted across a large code base. The best candidates for this kind of refactoring in web applications are classes related to date (Week, DateRange, DateIterator, ...), Money, Email, IPAddress, URLs, slugged Strings, integers used as bitmasks and many others.
Qafoo experts can kick-start your team with a continuous refactoring process. We can show you how to improve your source code quality on the go and help you to get rid of the big quality chuckholes in your construction site.
As soon as you detect business rules in your code that operate on primitive strings, integers or PHPs Date objects (they are not too powerful) you should think about extracting a value. If you wan't to avoid creating tons of object you can wait for 3-5 different rules on the same kind of primitive type or the same rule spread in 3-5 locations.
Stay up to date with regular new technological insights by subscribing to our newsletter. We will send you articles to improve your developments skills.
Xu on Tue, 16 May 2017 23:57:15 +0200
it is a typo on your title?
Link to commentshould it be "Extracting"?
Damian Dziaduch on Fri, 15 Dec 2017 13:22:26 +0100
This is great
Link to comment