The developer forbidden word : "and"

Devops

When we read code, sometimes we feel that something is bad. That the code should be written in another way. That's what Martin Fowler called "Bad smells in code" in his refactoring book.

There is something that really smells bad for me. This is the "And" word. Whether it is between a method name, a class name or a comment, it is always a bad sign.

Coupled Different stuffs

Most of the time, when I see the "and" word, the method or the object is trying to do two different things at the same place. The duplication is not so far away.

You should prevent that behavior. Each service, entity or method must have a single responsibility. In that way, it is easier to understand what each class is doing and to reuse your methods elsewhere.

Let's see an example :

function prepareOrder(Order $order)  
{
    if ($order->contains('sandwich') and $command->contains('coffee')) {
        prepareSandwichAndCoffee();
    } else if ($command->contains('sandwich')) {
        prepareSandwich();
    }
}

public function prepareSandwichAndCoffee()  
{
    cutBread();
    turnOnTheCoffeeMachine();
    putMeatInSandwich();
    putVegetableInSandwich();
    makeCoffee();
    packageTheSandwich();
}

public function prepareSandwich()  
{
    cutBread();
    putMeatInSandwich();
    putVegetableInSandwich();
    packageTheSandwich();
}

In that example, there is no reason to make a sandwich and a coffee in the same method. Those are two different things.

We can see where duplication appears when you create coupled code. If we want to add cheese in our sandwich, we have two functions to change. If we forget one, there will be bugs.

This duplication happened because the method is too much specific. When your code is not generic enough, you can't use it in a different contexts. So your are tempted to write it again.

In that situation, since there is no reason for that method to exist, we should delete it. In our case, we can do it three steps

First step : Reorganize the method

public function prepareSandwichAndCoffee()  
{
    cutBread();
    putMeatInSandwich();
    putVegetableInSandwich();
    packageTheSandwich();

    turnOnTheCoffeeMachine();
    makeCoffee();
}

Second step : Extract methods

public function prepareSandwichAndCoffee()  
{
    prepareSandwich();
    prepareCoffee();
}

public function prepareCoffee()  
{
    turnOnTheCoffeeMachine();
    makeCoffee();
}

public function prepareSandwich()  
{
    cutBread();
    putMeatInSandwich();
    putVegetableInSandwich();
    packageTheSandwich();
}

Third step : Delete the method

function prepareOrder(Order $order)  
{
    if ($order->contains('sandwich') and $command->contains('coffee')) {
        prepareSandwich();
        prepareCoffee();
    } else if ($command->contains('sandwich')) {
        prepareSandwich();
    }
}

public function prepareCoffee()  
{
    turnOnTheCoffeeMachine();
    makeCoffee();
}

public function prepareSandwich()  
{
    cutBread();
    putMeatInSandwich();
    putVegetableInSandwich();
    packageTheSandwich();
}

Optional step : Reorganize the calling context

function prepareOrder(Order $order)  
{
    if ( $command->contains('coffee')) {
        prepareCoffee();
    }
    if ($command->contains('sandwich')) {
        prepareSandwich();
    }
}

Give a meaning to what you do

Of course, there is some situation where we shouldn't split the methods in two others. Maybe because the two elements are tightly coupled or even just because there are used together in a lot of different places.

In this case, you have to ask yourself "Why do I need to do these two different computations at the same place?". If the answer is "Because the product owner wants it" or "I don't know", you are in the first case and you must split the function.

If you find a valid answer, use it to rename your method or your object.

Let's see an example :

function prepareOrder(Order $order)  
{
    if ($order->contains('menu A')) {
        prepareSandwichAndCake();
    }
}

// Called in a lot of different places
function prepareSandwichAndCake()  
{
    makeSandwich();
    takeAPieceOfCake();
    putItInTheSameBag();
}

Like in the first example, we have a method using the "And" word. This method doesn't seem to be over specific as it is used in different places. So we should ask "For what reason do I need to make a sandwich and take a cake at the same place?".

As you have maybe already seen, the reason is "Because they are in the same menu". As a result, we can rename the method prepareMenuA.

There are two benefits to this simple refactoring :

  • The reader can now understand why we created that function
  • If we want to add a drink to the menu A, we do not have to rename the function prepareSandwichCakeAndDrink.

More generally, you should named your items in a generic way and the asclose to the domain as possible.

The only exception

There is one exception where you can do two different things at the same place : for optimization. But be really careful with it.

Sometimes I have heard "I put it in the same function because it's faster". This is definitely not a good reason to make badly designed code. It is what we call Premature Optimization.

The only valid reason is "Because I did it well but it was too slow. So I changed it to make it faster".

Optimization has a big cost on readability and maintainability. Wait to be sure the block of code slows down your application before you optimize it.