Avoid output arguments(have no side effects)

Question

I'm reading 'Clean Code' from Robert C. Martin and I can't fully understand the 'Have No Side Effects' and 'Output Arguments' sections on pages 44 to 45.

In the 'Have No Side Effects' section it is stated that changes passed to parameters of a method are considered side effects and should not be done.

In the 'Output Arguments' section, it is stated that the parameters of a method should not be mutated. A method should only change the state of its owning object if some state has to be changed.

I do understand that side effects and output arguments can lead to confusing behavior and bugs if the method which does specify this behavior is called by a client who is not fully aware of that. Also, this is problematic when working in a multi-thread environment.

But with respect that the 'Clean Code' book is written based on examples with Java, I'm confused.

Let's take a look at an example. Let's assume we have a player class:

public class Player {
    private int healthPoints;
    private boolean alive = true;

    public Player(int healthPoints) {
        if(healthPoints < 1) {
            throw new IllegalArgumentException();
        }
        this.healthPoints = healthPoints;
    }

    public boolean isAlive() {
        return this.alive;
    }

    public void fight(Player otherPlayer) {
        //do some fighting which will change this instance
        // but also the otherPlayer instance
    }
}

If we now call the following function:

player1.fight(player2);

This will change the status of player1 and also the status of player2. This is discouraged by Robert C. Martin in most cases. But in reality, I see this pattern a lot. The most Java programs really on mutation and objects are changed outside the scope in which they have been created.

If we create a battle which changes both players, it is even worse because now the other object mutates two parameters inside its method:

battle.fight(player1, player2)

Do I miss something here? When is it ok to mutate parameters inside a method(in Java)? I asked a quite similar question some time ago(Is mutating object-parameters in a method(in Java) a bad practice?).

Calisaya

You miss one critical point of uncle Bob: those are guidelines, not hard rules. Bend them if necessary. After all, exactly this is your job: decide when to use which technique. As to the presented problem: one could give the Battle-objects two Player-properties and then just call .fight() on the Battle-object. That way, the Battle-object would only mutate its own state, i.e. its Players. – Nicotine

You should take it as a general guideline, not an absolute rule. The return type of the method and its name are hugely important to not deceive the caller. Something that you should absolutely avoid is something like boolean isStrongerThat(Player other) { boolean result = this.points > other.points; this.points++; other.points--; return result; }. The caller, based on the return type of the method and its name, expects no side-effect from such a method, but it changes the state of the two players, which is unexpected. – Tisza

A good real-life example of what you shouldn't do is the JavaScript method Array.splice(). It does two things at once (removing and adding), it has an awful name, it modifies the receiver array, but also returns it although the caller obviously already has a reference to it, thus leading the developer to think it returns a copy although it modifies the array. – Tisza

Game examples make poor programming examples, as game programming has a very different ruleset than general purpose programming. You end up with examples like class Sword extends Weapon, which has nothing to do with reality. – Cordie

Robert Martin is absolutely right. I hate when I face arguments mutation because it forces me to end up reading method's code almost necessarily. – Romansh

Answer 1

At the end of the day, these rules exist to make programmers' lives easier, by eliminating unexpected behaviour. They help human beings reason about programs. As such, the critical thing is to make sure that you can look at a line of code and understand what it does.

If I, as a programmer, see the line:

player1.fight(player2);

I would expect both player1 and player2 to be mutated in some way. This also applies to Arrays.sort(array);, which comes as part of the language.

However, if in your isAlive() function, you performed some mutation (possibly killing players with sub-zero health), that in my opinion, would be bad practice, since I, as the programmer, expect a function called isFoo() or getFoo() to simply return data about the object without any mutation.

At the end of the day, these guidelines are there to make your life easier when you come back and look at your code in a year's time, so if you're in doubt, ask yourself what you would expect a method with the same signature to do (i.e. what do I expect public void setAge(int age); to do? Does it do what I expect?)

P.S.

Some languages (e.g. Haskell) do not allow any mutation at all. Excluding IO and randomness, the only way a function can affect a program is through its return value. Worth checking out if you want to learn how "programming without side-effects" can be done.

Fibula