Wednesday, December 30, 2015

SOLID Principles: SRP and DIP (1 of 4)

In this series I'll explain five of the core O.O. design principles. Each entry will have an Agile Link section describing how the explained principle(s) relates to Agile thinking.

Single responsibility principle (SRP)

The SRP states that elements (classes and methods), should have only one reason to change. They must be focused on a single task, anything related to that task must be extracted as services / helpers and need to be placed inside other components, called dependencies.

This post will cover another core principle:

Dependency inversion principle (DIP)

Simply put, this principle says that both high and low level components should depend on abstractions (contracts), not on details.

Code sample

Code samples are on github repo lsolano/blog.solid.demo
This code sample (java SE 1.8), is a fake "ClientsReportBuilder" component, the ideal responsibility for this guy is to build a "Clients Report" pulling the data from underlying persistence mechanism and finally formatting it using one of the following (output formats): XML, JSON, or CSV. Code comments are omitted to keep it as short as possible.

This is the initial (dirty) version, the behavior is defined by the contract ClientsReportBuilder, and implemented by DirtyClientsReportBuilder. This version of the method buildNewClientsReports has around 40 lines of code (LOC). It has a very straitforward logic:
  • First, it does some validations for method's parameters.
  • Second, it instantiates a known version of the repository ClientsRepository (InMemoryClientsRepository), and uses it to get all clients with sign-on date within the given date range.
  • Third, it examines the format parameter and for each possible value builds the resulting string to return. Also if the format is not supported it throws and exception.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
import java.util.Date;
import java.util.stream.Stream;

public class DirtyClientsReportBuilder implements ClientsReportBuilder {

  @Override
  public String buildNewClientsReport(final Date from, final Date to, final OutputFormat format) {
    if (from == null || to == null || format == null) {
      throw new IllegalArgumentException("Arguments can not be null.");
    }

    if (from.compareTo(to) > 0) {
      throw new IllegalArgumentException(String.format("Invalid date range [%s, %s].", from, to));
    }

    final ClientsRepository clientsRepo = new InMemoryClientsRepository();

    final Stream<Client> clients = clientsRepo.getBySignOnDate(from, to);
    final StringBuffer sb = new StringBuffer();
    switch (format) {
    case XML:
      sb.append("<report>");
      clients.parallel()
          .forEach(client -> sb.append("<client><name>").append(client.getName()).append("</name></client>"));
      sb.append("</report>");
      break;

    case JSON:
      sb.append('[');
      clients.parallel().forEach(client -> sb.append("{ \"name\": \"").append(client.getName()).append("\" },"));
      sb.deleteCharAt(sb.length() - 1);
      sb.append(']');
      break;

    case CSV:
      sb.append(String.format("email,name%n"));
      clients.parallel().forEach(client -> sb.append(client.getEmail()).append(",").append(client.getName()));
      break;

    default:
      throw new IllegalArgumentException(String.format("Format not supported '%s'.", format));
    }

    return sb.toString();
  }
}

What is wrong with this code: SRP?

  1. At the class level: this class is supposed to "build report(s)" but is doing a little more: it knows to much about the repository used, and also knows about how to handle all output formats. This means that:
    1. If a new format is added, this class must change
    2. If a different implementation of the repository (flat file, RDB, No-SQL DB, ...), is used this class must change
  2. At the method level: this method is supposed to build the report using helper entities within its class or as outside services. We can count up to 3 responsibilities here:
    1. The parameters validation logic,
    2. Pulling out clients from repository, and
    3. Building the report using the proper format
In order to fix these violations we must split as much as possible all things done by the dirty class.

What is wrong with this code: DIP?

  1. This class depends on the ClientsRepository but actually has to much information about it. It knows about the implementation details.
  2. Conceptually, it must depend on a "formatter" component, actually spread inside the switch statement.
To fix the violations to the DI principle we must extract these two dependencies and define contracts (abstractions) so both high and low level components can interact knowing as little as possible about each other. Just to be clear, in this example the high level component is the ClientsReportBuilder and the low level ones are the repository and the (not yet created) formatter. 

Cleaning the code

Honoring SRP

At the class level the helper responsibilities where extracted:
  • A dependency to the repository contract was introduced to the, new and only, constructor
  • A dependency to builder / factory function for the formatter component, which will take the Client's collection (stream) and return the final string with the proper format.
At the method level, the following changes were made:
  • The parameters validation logic was extracted into a private method
  • The formatter was extracted to a helper factory function using the Optional pattern. If the format is invalid (not managed yet), the Option will be empty
  • The method was reduced from around 40 lines to just 12 lines of code.
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
import java.util.Date;
import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Stream;

public class CleanClientsReportBuilder implements ClientsReportBuilder {

  private final Function<OutputFormat, Optional<Function<Stream<Client>, String>>> formattersFactory;
  private final ClientsRepository clientsRepo;

  public CleanClientsReportBuilder(
      final Function<OutputFormat, Optional<Function<Stream<Client>, String>>> formattersFactory,
      final ClientsRepository clientsRepo) {
    this.formattersFactory = formattersFactory;
    this.clientsRepo = clientsRepo;
  }

  @Override
  public String buildNewClientsReport(final Date from, final Date to, final OutputFormat format) {
    validateReportParameters(from, to, format);

    final Stream<Client> clients = clientsRepo.getBySignOnDate(from, to);

    final Optional<Function<Stream<Client>, String>> formatter = this.formattersFactory.apply(format);

    if (formatter.isPresent()) {
      return formatter.get().apply(clients);
    }

    throw new IllegalArgumentException(String.format("Format not supported '%s'.", format));
  }

  private void validateReportParameters(final Date from, final Date to, final OutputFormat format) {
    if (from == null || to == null || format == null) {
      throw new IllegalArgumentException("Arguments can not be null.");
    }

    if (from.compareTo(to) > 0) {
      throw new IllegalArgumentException(String.format("Invalid date range [%s, %s].", from, to));
    }
  }
}

Honoring the DIP

To use this version of the component we need to do the following (see tests on GitHub repository).

1
2
3
final ClientsReportBuilder clientReportBuilder = 
 new CleanClientsReportBuilder(ClientsReportFormatter.buildFormattersFactory(), 
          new InMemoryClientsRepository());

Basically, we set both dependencies using the constructor's parameters. With a good IOC tool you just inject those into the component. Here both dependencies were extracted and supplied, achieving the following goals:

  • The report builder component knows nothing about the implementation of the repository. It only knows about the contract (using a java interface).
  • The report builder component knows nothing about the formatter components family. If only requires a factory function that will give Nothing or an actual formatter function.


Agile Link

I'll elaborate the code link to agile thinking using the Agile Principles from the manifesto.
Welcome changing requirements, even late in development. Agile processes harness change for the customer's competitive advantage.
With the final implementation is easy to add a new output format. Also we can change the repository implementation without impacting the report builder. Adding / fixing the validation logic is easy because is encapsulated within a single, short method.

Agile processes promote sustainable development. ... [We] should be able to maintain a constant pace indefinitely.
With a clean code base we avoid the "maintenance hell". We avoid working extra hours and burning the team in the process. Remember this is job, we must not die like heroes in the process, if that what you want join the army :)

Continuous attention to technical excellence and good design enhances agility.
"Excellence" is a relative term, but code's final version is more elegant than the first. Surely, someone in the future can improve the code even further but is our job to let the code as clean and simple as our current knowledge level allows.

Next on this series

SOLID Principles: Open/closed principle (OCP) 

Thursday, December 24, 2015

Reflections on Clean Code: Team's Performance

XP-agra for flaccid Scrum teams (Doctor's Choice)


Keep reading if you find yourself or your team in one of the following situations:
  1. You no longer have confidence in your estimates (time, effort, risks)
  2. You have 'owners' for certain software components
  3. Your regression test takes more than half a day
  4. A very high rate of bugs (defects) are discovered in production by your users
  5. In order to find the piece of code to change or the place to add a few new lines you need to spend hours digging the code base
  6. Sometimes you touch Component A and the change affects Components B to Z, and you discover that on late test cycles or live in production
  7. With every iteration you feel that the time to perform minor changes is growing exponentially
  8. Everybody fears the release date, and stay alert the next week or two expecting the next bomb coming from production
  9. Your team is doing Scrum (or so you believe)

If any of the described situations is happening to you be advised: your team is suffering from the disease called "Flaccid Scrum" (FlaccidScrum). This illness causes your performance to, gradually, decrease until you reach the zero productivity (velocity) point. By that time the only possible solution is to dump the code base (put the product in the trashcan), and start over with a greenfield project.

But, fear no more, you can be cured with a simple solution: just take XP-agra the Doctor's Choice for flaccid scrum implementations. You need to start with solid engineering practices as proposed by eXtreme Programming.


The basic problem is that a flaccid scrum team is constantly neglecting the need for internal quality (not just functional quality) in the code base. A team with such a problem is compromising (1) the quality vertex from the following triangle:

Solid Scrum (with XP) mindset

Let's examine each vertex, in the context of "normal" Agile team:
  • The time one is the easiest, it is fixed by one of the core concepts of agile methodologies "Time Boxing"
  • The scope vertex must be flexible in order have really time boxed iterations, and
  • The quality vertex must be fixed in order to maintain a sustainable velocity (performance) by the team.

Flaccid Scrum mindset 

This is the mindset, respecting each vertex, for a "flaccid" Scrum team:

  • The time one remains fixed (time boxed)
  • The scope vertex is fixed giving a false sense of commitment and "performance" to the Product Owner and the customer, and
  • The quality vertex, by necessity, becomes flexible in order to allocate all committed scope inside the time boxed iteration.

The Jenga Tower

By compromising the internal quality the team is shooting himself in the foot because the performance will be gradually degraded. A poor quality code base is difficult to maintain, a headache for the team and for the customers. It becomes very fragile, with each change a new crack arise. With each crack a quick patch is added. After a few iterations the code base has too many patches and becomes extremely hard to touch without breaking it: this is the final stage of a Jenga Tower before collapsing. Something like this: 
"Each block removed is then balanced on top of the tower, creating a progressively taller but less stable structure." (https://en.wikipedia.org/wiki/Jenga)


Practical steps towards solving the problem

Just to be crystal clear,  XP is not a pill, we really can just "take it" and the problem will be gone. In order to solve the underlying quality problem we need to take small, but solid actions. Here I'm only listing the suggested steps but details on each one will be left for upcoming posts:
  • First, we need to build awareness around the issue for all stakeholders:
    • Your team (complete): developers, testers, leaders, ...
    • Your product owner
    • Your customers: both inside and outside the organization
    • Capital investors / business owners
  • Second, improve the knowledge level of the people actually doing the work, because they need to change their bad habits and may require some sort of education / coaching / etc.
  • Finally, devise a plan covering short, mid, and large term actions for team to undertake. 
    • One of the first steps of this plan is to measure the current situation
    • Then, you need to define the KPIs that will be observed as process and quality improvement indicators
    • Pick the smallest steps possible, because you don't want to completely stop the team and you need each new practice to be assimilated by the team shifting the mindset a little bit each time. 

Monday, December 14, 2015

Reflections on Clean Code: Estimates

Reflections on Clean Code: Estimates

Day one of your iteration, your team is doing the planning game (planning poker, ideal hours, etc.) and everyone is reflecting about the effort needed to work a given requirement. We could have two scenarios:

One: With a Clean Code Base

Clean code estimates
Clean Code estimates



Two: With a Bad (dirty) Code Base

Bad Code estimates
Bad Code estimates


Imaging your code base as a tool’s repository, those tools are used to solve problems. Your job is to use the proper tool for each task, but you need to do that as quickly as possible. Now think what would happen if any time you enter the shop you find a mess of tools?

In order to do your job, first you need to find all required tools, you reach for the hammer, but find a screwdriver, then you find the drill on the level’s bucket, … (you fell me?)

After finding all required tools you start working on the piece of furniture, but you find that the last carpenter use nails where you expected screws. Also, he had use glue where fine nails where more appropriated. The hammer has grease on the handle and slips. The drill heads are out of order …

By the end of the day you are done with the piece, but you realize that building it from scratch will take half the time.

Too much bla, bla, bla: What should we do about this?

Put each piece / tool inside the proper drawer:

First, imagine your code base as a cabinet with many drawers, each one with a label. Depending on your design decisions labels will have words like (persistence, core logic, controllers, views, domain, tests, etc.); but no matter how your team decided to organize the code, each element must be inside the proper drawer.

You must put components where they belong in order in accordance to the team’s guidelines for the given application.


Leave pieces as clean as possible:

Now imagine a single file as the drill and its heads. Any time you use change heads you need to leave the set in proper order. Also, you should clean and grease moving parts regularly in order to extend the tool’s live and improve performance.
You must keep your components (files, classes, methods, etc.) as clean as possible both on the inside (implementation) and on the outside (public interface / contract).


Include the “cleaning” process in all your estimates:

Let me change the analogy for a moment: imaging what would happen if your preferred restaurant only estimates the cost of serving the dish to your table and cleaning up after you leave. But they “forget” to consider cost of dish-washing, kitchen cleaning cost and time, bathrooms cleaning time, etc.

So, the next time you’ll get there to eat your favorite dinner, they need to spend one hour just to start working on it because all elements were dirty and out of order. So you get served with one-hour delay. As they do all this in such a hurry, you find that the fork is dirty and the napkins have stains all over…

The same happens with software developers only estimating the “change” / “implementation” time,  and not considering the time needed to leave the solution as clean as they found it. Also they forgot the time to add / change existing documentation to reflect the changes. They do not consider the time for proper testing. Simply, they are leaving the shop dirty and tools out of order. When they need to “serve” another client they found the same problems as the lazy restaurant’s employees.

Subscribe to our mailing list

* indicates required
Email Format