Monday, June 19, 2017

The Patterns of the Antipatterns: Design / Testing

It's been a while since I have started my professional career as the software developer. Over the years I worked for many companies, on a whole bunch of different projects, trying my best to deliver them and be proud of the job. However, drifting from company to company, from project to project, you see the same bad design decisions made over and over again, no matter how old the codebase is or how many people have touched it. They are so widely present that truly become the patterns themselves, thereby destine the title of this blog: the patterns of the antipatterns.

I am definitely not the first one who identified them and certainly not the last one either (so let all the credits go to the original sources if any exist). They all come from the real projects, primarily Java ones, though other details should stay undisclosed. With that ...

Code first, no time to think

This is hardly sounds like a name of the antipattern but I believe it is. We all are developers, and unsurprisingly love to code. Whenever we have gotten some feature or idea to implement, we often just start coding it, without thinking much about the design or even not looking around if we could borrow / reuse some functionality which is already there. As such, reinventing the wheel all the time, hopefully enjoying at least the coding part ...

I am absolutely convinced that spending some time just thinking about high-level design, incarnating it in terms of interfaces, traits or protocols (whatever your favoring language is offering) is a must-do exercise. It won't take more time, really, but the results are rewarding: beautiful, well-thought solution at the end.

TDD is a set of the terrific practices to help and guide your through. While you are developing test cases, you are iterating over and over, making changes and with each step getting close to the right implementation. Please, adopt it in each and every project you are part of.

Occasional Doer

Good, enough philosophy, now we are moving on to a real antipatterns. Undoubtedly, many of you witnessed such pieces of code:

public void setSomething(Object something) {
    if (!heavensAreFalling) {
        this.something = something;
    }
}

Essentially, you call the method to set some value, and surely you expect the value to be set once the invocation is completed. However, due to presence of some logic inside the method implementation it may actually do nothing, silently ...

This is an example of really bad design. Instead, such interactions could be modeled using for example State pattern or at least just throwing an exception saying that the operation cannot be completed due to internal constraints. Yes, it may require a bit more code to be written but at least the expectations will be met.

The Universe

Often there is a class within the application which is referencing (or is referenced by) mostly every other class of the application. For example, all classes in the project must be inherited from some base class.

public class MyService extends MyBase {
  ...
}
public class MyDao extends MyBase {
  ...
}

In most cases, this is not a good idea. Not only it creates a hard dependency on this single class but also on every dependency this guy is using internally. You may argue that in Java every single class implicitly is inherited from Object. Indeed, but this is part of the language specification and has nothing to do with application logic, no need to mimic that.

Another extreme is to have a class which serves as a central brain of the application. It knows about every service / dao / ... and provides accessors (most of the time, static) so anyone can just turn to it and ask for anything, for example:

public class Services {
    public static MyService1 getMyService1() {...}
    public static MyService2 getMyService2() {...}
    public static MyService3 getMyService3() {...}
}

Those are universes, eventually they leak into every class of the application (it is easy to call static method, right?) and couple everything together. In more or less large codebase, getting rid of such universes is exceptionally hard.

To prevent such things to poison your projects, use dependency injection pattern, implemented by Spring Framework, Dagger, Guice, CDI, HK2, or pass the necessary dependencies through constructors or method arguments. It will actually help you to see the smells early on and address them even before they become a problem.

The Onionated Inheritance

This is a really fun and scary one, very often present in projects which expose and implement REST(ful) web services. Let us suppose you have a Customer class, with quite a few different properties, for example:

public class Customer {
    private String id;
    private String firstName;
    private String lastName;
    private Address address;
    private Company company;
    ...
}

Now, you have been asked to create an API (or expose through other means) which finds the customer but returns only his id, firstName and lastName. Sounds straightforward, isn't it? Let us do it:

public class CustomerWithIdAndNames {
    private String id;
    private String firstName;
    private String lastName;
}

Looks pretty neat, right? Everyone is happy but next day you get to work on another feature where you need to design an API which returns id, firstName, lastName and also company. Sounds pretty easy, we already have CustomerWithIdAndNames, only need to enrich it with a company. Perfect job for inheritance, right?

public class CustomerWithIdAndNamesAndCompany extends CustomerWithIdAndNames {
    private Company company;
}

It works! But after a week, another request comes in, where a new API also needs to expose the address property, anyway, you got the idea. So at the end you end up with dozen of classes which extends each other, adding properties here and there (like onions, hence the antipattern name) to fulfill the API contracts.

Yet another road to hell ... There are quite a few options here, the simplest one is JSON Views (here are some examples using Jackson) where the underlying class stays the same but different views of it could be returned. In case you really care about not fetching the data you don't need, another option is GraphQL which we have covered last time. Essentially, the message here is: don't create such onionated hierarchies, use single representation but use different techniques to assemble it and fetch the necessary pieces efficiently.

IDD: if-driven development

Most of real-world projects internally are built using pretty complex set of application and business rules, cemented by applying different layers of extensibility and customizations (if needed). In most cases, the implementation choice is pretty unpretentious and the logic is driven by a conditional statements, for example:

int price = ...;
if (calculateTaxes) {
    price += taxes;
}

With time, the business rules evolve, so do the conditional expression they are impersonated, becoming a real monsters at the end:

int price = ...;
if (calculateTaxes && (country == "US" || country == "GB") && 
    discount == null || (discount != null && discount.getType() == TAXES)) {
    price += taxes;
}

You see where I am going with that, the project is built on top IDD practices: if-driven development. Not only the code becomes fragile, prone to condition errors, very hard to follow, it is very scary to change as well! To be fair, you also need to test all possible combinations of such if branches to ensure they make sense and the right branch is picked up (I bet no one is actually doing that because this is a gigantic amount of tests and efforts)!

In many cases the feature toggles could be an answer. But if you get used to write such a code, please take some time and read the books about design patterns, test-driven development and coding practices. There are so many of them, below is the list I would definitely recommend:

There are so many great ones to add here, but those are a very good starting point and highly rewarding investment. Much better alternatives to if statements are going to fill you mind.

Test Lottery

When you hear something like "All projects in our organization are agile and using TDD practices", the idealistic pictures come in mind, you hope everything is done right by the book. In reality, in most projects things are very far from that. Hopefully, at least there are some test suites you could trust, or could you?

Please welcome the test lottery: the kingdom of flaky tests. Have you ever seen builds with random test failures? Like on every consecutive build (without any changes introduced) some tests suddenly pass but others are starting to fail? And may be one of ten builds may turn green (jackpot!) as stars finally got aligned properly? If no one cares, it becomes a new normal and every team member who joins the team is being told to ignore those failures, "they are failing for ages, screw them".

Tests are as important as the mainstream code you push into production. They need to be maintained, refactored and kept clean. Keep all your builds green all the time, if you see some flakiness or random failures, address them right away. It is not easy but in many cases you may actually run into real bugs! You have to trust your test suites, otherwise why do you need them at all?

Test Framework Factory

This guy is an outstanding one. It happens so often, it feels like every organization is just obligated to create own test framework, usually on top of existing ones. At first it sounds like a good idea, and arguably, it really is. Unfortunately, in 99% the outcome is yet another monstrous framework no one wants to use but is forced to because "it took 15 men / years to develop and we cannot throw it away after that". Everyone struggles, productivity is going down at the speed of light, and quality does not improve at all.

Think twice before taking this route. The goal to simplify testing of complex applications the organization is working on is certainly a good one. But don't fall into the trap of throwing people at it, who are going to spend few months or even years working on the "perfect & mighty test framework" in isolation, may be doing great job overall, but not solving any real problems. Instead, just creating new ones. If you decided to embark on this train anyway, try hard to stay focused, address the pain points at their core, prototype, always ask for feedback, listen and iterate ... And keep it stable, easy to use, helpful, trustful and as lightweight as possible.

Conclusions

You cannot build everything right from the beginning. Sometimes our decisions are impacted by pressure and deadlines, or the factors outside of our control. But it does not mean that we should let things stay like that forever, getting worst and worst every single day, please, don't ...

This post is a mesh of ideas from many people, terrific former teammates and awesome friends for good, credits go to all of them. If you happens to like it, you may be interested in checking out the upcoming post, where we are going to talk about architectural antipatterns.