SRP Example – Bowling Game

Browsing Uncle Bob’s blog, I’ve found this interesting post about teaching TDD with a practical example. It tries to show the principles of TDD while implementing a bowling game. A class diagram showing the mainly concepts of the game is presented. Here it is (click on the thumbnail to see a larger image):

Bowling Game

Some Java code of Game class is show below:

   1:public class Game
   2:{
   3:    private int rolls[] = new int [21];
   4:    private int currentRoll = 0;
   5:
   6:    public void roll(int pins)
   7:    {
   8:        rolls[currentRoll++] = pins;
   9:    }
  10:
  11:    public int score()
  12:    {
  13:        int score = 0;
  14:        int frameIndex = 0;
  15:        for(int frame = 0; frame < 10; frame++) {
  16:            if(isStrike(frameIndex)) {
  17:                score += 10 + strikeBonus(frameIndex);
  18:                frameIndex++;
  19:            }
  20:            else if(isSpare(frameIndex)) {
  21:                score += 10 + spareBonus(frameIndex);
  22:                frameIndex += 2;
  23:            }
  24:            else {
  25:                score += sumOfBallsInFrame(frameIndex);
  26:                frameIndex += 2;
  27:            }
  28:        }
  29:        return score;
  30:    }
  31:    .
  32:    .
  33:}

The Single Responsability Principle (SRP) states “there should never be more than one reason for a class to change”. Analysing carefully the Game class, you can note it has more than one reason to change. So, this class has more than one responsability. One is to keep track of the current frame and the other is to calculate the score. It sometimes is hard to see beacuse it’s difficult to detect a bad class design concerning SRP, because SRP is about implementation, not interface, as I’ve posted before. It’s bad for a class to have two responsabilities, because they become coupled. In the real world this king of coupling doesn’t exist, so in the computational world you can’t create this coupling. If we have to change a client of the BowlingGame class who depends only on the roll() operation and this change causes the BowlingGame class too, we would have to rebuild and retest another client of the BowlingGame class who depends on the score() operation. A better design would separate these responsabilities in different classes or maybe applying Interface Segregation Principle (ISP). I’ve changed a little this class design, ending up doing this (click ont the thumbnail to see a larger image):

Bowling Game

Notice that I have decoupled the clients from the BowlingGame in terms of interfaces. Now, those interfaces provide the clients the services they need, staying far away from the Game implementation.
I think this class design is better beacuse we have decoupled the concepts from each other concerning the whole application, separating in two interfaces. Notice the implementation of the two responsabilities still continues in the BowlingGame
class, but nobody need depend upon this class. Nobody will know it exists. The client who needs to know the bowling game score, will depend on the Scorer interface and another client who wants to register rolls will depend on the RollRegister interface. The implementation is far away from the client. I will change the code concerning this new design and I will post here later. As I’ve told, I think this design is better. Do you have an opinion or suggestion about this?

Applying the Law of Demeter

Have you ever been told about the Law of Demeter when developing object-oriented systems? This law states the following:

More formally, the Law of Demeter for functions requires that any method M of an object O may only invoke the methods of the following kinds of objects:

  1. itself
  2. its parameters
  3. any objects it creates/instantiates
  4. its direct component objects

In particular, an object should avoid invoking methods of a member object returned by another method.

Browsing the source code of a project, I found this Java code stretch:

   1:public class AccountHelper
   2:{
   3:    public void creditToAccount(Account account, double value)
   4:    {
   5:        double balance = account.balance();
   6:        double newBalance = balance + value;
   7:        account.setBalance(newBalance);
   8:    }
   9:
  10:    public boolean withdrawFromAccount(Account account,
  11:            double value)
  12:    {
  13:        double balance = account.balance();
  14:        double limit = account.limit();
  15:        double newBalance = balance;
  16:        boolean canWithdraw = false;
  17:        if(balance + limit >= value) {
  18:            newBalance = balance - value;
  19:            canWithdraw = true;
  20:        }
  21:        account.setBalance(newBalance);
  22:        return canWithdraw;
  23:    }
  24:}

This code snippet doesn’t follow demeter’s law. And it’s hiding a responsability that would have been better in the Account class. If the Account knows its balance and limit, why putting the credit and withdraw behavior in a helper class? If we follow the Specialist grasp pattern, the responsability has to be at the class with the specific knowledge. So, I’ve improved the design to this new one Java code:

   1:import java.math.BigDecimal;
   2:
   3:public class Account
   4:{
   5:    private BigDecimal balance;
   6:    private BigDecimal limit;
   7:
   8:    public Account(double balance)
   9:    {
  10:        this.balance = new BigDecimal(balance);
  11:    }
  12:
  13:    public Account(double amount,
  14:        double limit)
  15:    {
  16:        this(amount);
  17:        this.limit = new BigDecimal(limit);
  18:    }
  19:
  20:    public void credit(double value)
  21:    {
  22:        balance = balance.add(new BigDecimal(value));
  23:    }
  24:
  25:    public boolean withdraw(double quantity)
  26:    {
  27:        if(canWithdraw(quantity)) {
  28:            balance.subtract(new BigDecimal(quantity));
  29:            return true;
  30:        }
  31:        return false;
  32:    }
  33:
  34:    public double balance()
  35:    {
  36:        return balance.doubleValue();
  37:    }
  38:
  39:    private boolean canWithdraw(double quantity)
  40:    {
  41:        return (balance.doubleValue() + 
  42:                limit.doubleValue() >= quantity)
  43:                ? true
  44:                : false;
  45:    }
  46:
  47:    public void setBalance(double balance)
  48:    {
  49:        this.balance = new BigDecimal(balance);
  50:    }
  51:
  52:    public double limit()
  53:    {
  54:        return limit.doubleValue();
  55:    }
  56:}

So, the initial Java code stretch become this one:

   1:public class AccountHelper
   2:{
   3:    public void credit(Account account, double value)
   4:    {
   5:        assert account != null;
   6:        account.credit(value);
   7:    }
   8:
   9:    public boolean withdraw(Account account, double value)
  10:    {
  11:        assert account != null;
  12:        return account.withdraw(value);
  13:    }
  14:}

This code above is better because we hide from the clients the Account class internal structure (clients don’t need to know the existence of suborders). Moreover, if the internal structure of account is intended to change, less clients would suffer with those changes (actually, if the Account class changes, only it would have to receive the changes and its clients would have to stay far away from its changes. It’s why Object Oriented design is focused in behavior instead of state). This nice phrase resumes demeter’s law: “The resulting software tends to be more maintainable and adaptable. Since objects are less dependent on the internal structure of other objects, object containers can be changed without reworking their callers”.