Refactoring to patterns. Yet another TDD example coded in Delphi

Long overdue here is my second article about Test Driven Development (TDD) in Delphi. This is a continuation of TDD in Delphi: The Basics, another post that I wrote a few months earlier. 

I would like to focus in a particular step within the TDD cycle: refactoring the code. Refactoring means optimizing, cleaning, shortening, beautifying, styling (put your own word here) the code without breaking the functionality; that is, without breaking your unit tests.

 
By having unit tests in place before refactoring, you guarantee that the changes to the code are safe. Refactoring can introduce bugs. To avoid those bugs you need your unit tests in place.

Refactoring can introduce something else: refactoring can introduce design patterns into your code. That means you don’t have to introduce the design patterns up-front, since your code can evolve from a “very rustic implementation” to a “pattern oriented implementation”. This is referred as “refactoring to patterns”. If you are interested on the topic, I advise you to read Refactoring To Patterns by Joshua Kerievsky.

I’ll take the chess game as the base to my example. For simplicity, I’ll just refer to a couple of pieces: the knight and the bishop. In this example, I will just focus in refactoring some code with unit tests already in place. A detailed walk-through for the TDD cycle can be found in my previous article, which is also based on the chess game.  

  
The code is easy enough to be self-explanatory: basically, there is a class hierarchy in which TPiece is the base class from which TKnight and TBishop derive. Take a quick look:

unit ChessGame;

interface

type

 TPiece = class
 private
   FX,
   FY: Byte;
 public
   constructor Create(aX, aY: Integer);
   function IsWithinBoard(aX, aY: Integer): Boolean;
 end;

 TBishop = class (TPiece)
 public
   function CanMoveTo(aX, aY: Byte): Boolean;
   function isValidMove(aX, aY: Byte): Boolean;
 end;

 TKnight = class(TPiece)
  public
    function CanMoveTo(aX, aY: Byte): Boolean;
    function isValidMove(aX, aY: Byte): Boolean;
 end;


implementation

{ TPiece }

constructor TPiece.Create(aX, aY: Integer);
begin
  inherited Create;
  // TODO: check that this assignment is valid.
  // Not now, ok? :-)
  FX:= aX;
  FY:= aY;
end;

function TPiece.IsWithinBoard(aX, aY: Integer): Boolean;
begin
  Result:= (aX > 0) and
           (aX < 9) and
           (aY > 0) and
           (aY < 9);
end;

{ TKnight }

function TKnight.isValidMove(aX, aY: Byte): Boolean;
var
  x_diff,
  y_diff: Integer;
begin
  x_diff:= abs(aX - FX) ;
  y_diff:= abs(aY - FY) ;

  Result:= ((x_diff = 2) and (y_diff = 1))
                         or
           ((y_diff = 2) and (x_diff = 1));
end;

function TKnight.CanMoveTo(aX, aY: Byte): Boolean;
begin
  Result:= IsWithinBoard(aX, aY) and
           IsValidMove(aX, aY);
end;

{ TBishop }

function TBishop.isValidMove(aX, aY: Byte): Boolean;
begin
  Result:= abs(aX - FX) = abs(aY - FY);
end;

function TBishop.CanMoveTo(aX, aY: Byte): Boolean;
begin
  Result:= IsWithinBoard(aX, aY) and
           IsValidMove(aX, aY);
end;

end. 


/////////////////////////////////////////////

 
unit TestChessGame;

interface

uses
  TestFramework, ChessGame;

type
  // Test methods for class TPiece
  TestTPiece = class(TTestCase)
  strict private
    FPiece: TPiece;
  public
    procedure SetUp; override;
    procedure TearDown; override;
  published
    procedure TestIsWithinBoard;
  end;

  // Test methods for class TBishop
  TestTBishop = class(TTestCase)
  strict private
    FBishop: TBishop;
  public
    procedure SetUp; override;
    procedure TearDown; override;
  published
    procedure TestCanMoveTo;
    procedure TestisValidMove;
  end;

  // Test methods for class TKnight
  TestTKnight = class(TTestCase)
  strict private
    FKnight: TKnight;
  public
    procedure SetUp; override;
    procedure TearDown; override;
  published
    procedure TestCanMoveTo;
    procedure TestisValidMove;
  end;

implementation

procedure TestTPiece.SetUp;
begin
  FPiece := TPiece.Create(4, 4);
end;

procedure TestTPiece.TearDown;
begin
  FPiece.Free;
  FPiece := nil;
end;

procedure TestTPiece.TestIsWithinBoard;
begin
  //Test trivial (normal) workflow
  Check(FPiece.IsWithinBoard(4, 4));

  //Tests boundaries
  Check(FPiece.IsWithinBoard(1, 1));
  Check(FPiece.IsWithinBoard(1, 8));
  Check(FPiece.IsWithinBoard(8, 1));
  Check(FPiece.IsWithinBoard(8, 8));

  //Test beyond the boundaries
  CheckFalse(FPiece.IsWithinBoard(3, 15));
  CheckFalse(FPiece.IsWithinBoard(3, -15));
  CheckFalse(FPiece.IsWithinBoard(15, 3));
  CheckFalse(FPiece.IsWithinBoard(15, 15));
  CheckFalse(FPiece.IsWithinBoard(15, -15));
  CheckFalse(FPiece.IsWithinBoard(-15, 3));
  CheckFalse(FPiece.IsWithinBoard(-15, 15));
  CheckFalse(FPiece.IsWithinBoard(-15, -15));
end;

procedure TestTBishop.SetUp;
begin
  FBishop := TBishop.Create(4, 4);
end;

procedure TestTBishop.TearDown;
begin
  FBishop.Free;
  FBishop := nil;
end;

procedure TestTBishop.TestCanMoveTo;
begin
  // Hey developer, indulge me here: believe
  // that I fully wrote the code for this
  // test already before writing anything else.
end;

procedure TestTBishop.TestisValidMove;
begin
  // Hey developer, indulge me here: believe
  // that I fully wrote the code for this
  // test already before writing anything else.
end;

procedure TestTKnight.SetUp;
begin
  FKnight := TKnight.Create(4, 4);
end;

procedure TestTKnight.TearDown;
begin
  FKnight.Free;
  FKnight := nil;
end;

procedure TestTKnight.TestCanMoveTo;
begin
  // Hey developer, indulge me here: believe
  // that I fully wrote the code for this
  // test already before writing anything else.
end;

procedure TestTKnight.TestisValidMove;
begin
  // Hey developer, indulge me here: believe
  // that I fully wrote the code for this
  // test already before writing anything else.
end;

initialization
  // Register any test cases with the test runner
  RegisterTest(TestTPiece.Suite);
  RegisterTest(TestTBishop.Suite);
  RegisterTest(TestTKnight.Suite);
end.

Note that the method CanMoveTo is duplicated in both TKnight  and TBishop; that’s not nice isn’t it? In order to fix this, we can pull-up the CanMoveTo method to the TPiece base class. Note this now: the CanMoveTo has now become a “template method”; because is a general algorithm applicable to all kind of chess pieces (TKnight ,TBishop, etc) .

This general algorithm has deferred some steps to be implemented in the subclasses; I mean, the isValidMove method is still coded in the subclasses. Isn’t this a beauty?  You have now refactored your code and when doing so, you have introduced the Template Method Design Pattern.

What’s even best, (don’t forget this because it is a key part): is that we can guarantee that our fancy refactoring didn’t break our pre-existing functionality. Why? Because we had unit tests in place written a long time ago. Writing unit test from the beginning gives a huge peace of mind to the developer :-) See the new refactored code below:

unit ChessGameRefactored;

interface

type

 TPiece = class
 private
   FX,
   FY: Byte;
 public
   constructor Create(aX, aY: Integer);
   function IsWithinBoard(aX, aY: Integer): Boolean;

   function CanMoveTo(aX, aY: Byte): Boolean;
   function isValidMove(aX, aY: Byte): Boolean; virtual; abstract;
 end;

 TBishop = class (TPiece)
 public
   function isValidMove(aX, aY: Byte): Boolean; override;
 end;

 TKnight = class(TPiece)
  public
    function isValidMove(aX, aY: Byte): Boolean; override;
 end;


implementation

{ TPiece }

constructor TPiece.Create(aX, aY: Integer);
begin
  inherited Create;
  // TODO: check that this assignment is valid.
  // Not now, ok? :-)
  FX:= aX;
  FY:= aY;
end;

function TPiece.IsWithinBoard(aX, aY: Integer): Boolean;
begin
  Result:= (aX > 0) and
           (aX < 9) and
           (aY > 0) and
           (aY < 9);
end;

function TPiece.CanMoveTo(aX, aY: Byte): Boolean;
begin
  Result:= IsWithinBoard(aX, aY) and
           IsValidMove(aX, aY);
end;

{ TKnight }

function TKnight.isValidMove(aX, aY: Byte): Boolean;
var
  x_diff,
  y_diff: Integer;
begin
  x_diff:= abs(aX - FX) ;
  y_diff:= abs(aY - FY) ;

  Result:= ((x_diff = 2) and (y_diff = 1))
                         or
           ((y_diff = 2) and (x_diff = 1));
end;

{ TBishop }

function TBishop.isValidMove(aX, aY: Byte): Boolean;
begin
  Result:= abs(aX - FX) = abs(aY - FY);
end;

end.

Conclusion, in addition to all the cool things of TDD there’s the possibility of refining your design not up-front, but when refactoring your code. Design patterns can be introduced at any time and we know that such introduction, if late, is not going to break our logic, because we have unit tests in place to prevent that from happening.

Some related reading below:

CodeProject

Template Method Design Pattern in Delphi. A working example

The Template Method Pattern is very easy to understand and implement. Here’s the definition borrowed from Design Patterns: Elements of Reusable Object-Oriented Software book:

“Define the skeleton of an algorithm in an operation, deferring some steps to subclasses. Template Method lets subclasses redefine certain steps of an algorithm without changing the algorithm's structure.”

Let’s try to understand the pattern walking through a simple example. For implementing the solution we’ll use Delphi XE, but it should work for previous versions as well.

This is the wording of the example:

Design an application that allows drawing different styles of houses (ei. country house, city house) using ASCII art [1].

In the following image there’s a prototype of the user interface.

The GUI is composed by a form, a memo [2], two buttons, a label and a combo box. Using the combo box you can choose whether your house will have a chimney or not. Clicking the buttons will result in an ASCII house printed out in the memo area. In the example above, the Draw Country House was clicked. If you click the other button, you will get city house like the one below:

What happens if you select “No” in the combo box? Try it and let me know :-) Here’s the link to the EXE. This EXE is harmless; I am not a bad guy: no Trojans, no viruses, just Delphi :-)

Now, what classes do we need to make this work? Take a look at the following class diagram and try to make sense of it:

We have an abstract superclass THouse, which contains four abstract (pure virtual) methods: BuildFloor, BuildWalls, BuildRoof and BuildChimney. These methods are implemented in the descending classes TCountryHouse and TCityHouse. Pay special attention to the method BuildIt in THouse. This method is an instance method that is inherited by both concrete subclasses: TCountryHouse and TCityHouse.  

I want to be more detailed:

The BuildIt method is implemented just once in the class THouse. The subclasses TCountryHouse and TCityHouse don’t have to implement the BuildIt method.

The BuildFloor, BuildWalls, BuildRoof and BuildChimney methods have no implementation under the THouse class. The TCountryHouse has one implementation of these methods, while TCityHouse have a different implementation of them.

    

This is how the BuildIt method looks like:

function THouse.BuildIt: string;

begin

  Result:= BuildRoof  +  //Step 1

           BuildWalls +  //Step 2

           BuildFloor;   //Step 3




  if FHasChimney then

    Result:= BuildChimney+ Result;  //Step 4 (conditional)

end;

This method defines a general algorithm to build a house…it doesn’t take into consideration the kind of house that’s been built. Notice that the steps for building the roof, walls, floor and chimney (if any) have been deferred to the subclasses. The BuildIt method is known as a template method that gives the name to this pattern.

Let me give you an idea of the implementation of the BuildFloor method. This method is implemented directly by the subclasses. Take a look:

function TCountryHouse.BuildFloor: string;

begin

  Result:=




  '~~~~~"   "~~~~~~~~~~~~~~~~~~~~~~~~  ';

end;




function TCityHouse.BuildFloor: string;

begin

  Result:=




  '******************____****************'#13#10 +

  '**************************************';

end;

Notice that the implementation of the BuildFloor method is different in both subclasses TCountryHouse and TCityHouse.

For the implementation of the remaining methods refer to reference [5]. For the full source code refer to [3].

In conclusion, the template method pattern needs an abstract superclass to implement a template method (common for all subclasses). The steps within this template method are deferred into methods that are implemented by the concrete subclasses. Did you get it? :-)

Do you want to know more about design patterns? The books in reference [4] are just what you need.

References:

[1] I am borrowing the ASCII designs from Asciiworld.com : House.

[2] Make sure to use a fixed-width font (Courier, Monaco, Courier New, Lucida Console, etc.) for the memo, otherwise the drawing will look fuzzy.

[3] Get the full source code of this example here.

[4] Books on Design Patterns:

[5] The full source code of the unit:

unit TemplatePatternExample;

interface
type
  THouse = class
  private
    FHasChimney: Boolean;
  public
    constructor Create;

    property HasChimney: Boolean read  FHasChimney
                                 write FHasChimney;

    function BuildFloor: string;   virtual; abstract;
    function BuildWalls: string;   virtual; abstract;
    function BuildRoof: string;    virtual; abstract;
    function BuildChimney: string; virtual; abstract;

    function BuildIt: string;
  end;

  TCountryHouse = class(THouse)
  public
    function BuildFloor: string;   override;
    function BuildWalls: string;   override;
    function BuildRoof: string;    override;
    function BuildChimney: string; override;
  end;

  TCityHouse = class(THouse)
  public
    function BuildFloor: string;   override;
    function BuildWalls: string;   override;
    function BuildRoof: string;    override;
    function BuildChimney: string; override;
  end;

implementation

{ THouse }

constructor THouse.Create;
begin
  inherited Create;
  FHasChimney:= True;
end;

function THouse.BuildIt: string;
begin
  Result:= BuildRoof  +  //Step 1
           BuildWalls +  //Step 2
           BuildFloor;   //Step 3

  if FHasChimney then
    Result:= BuildChimney+ Result;  //Step 4 (conditional)
end;

{ TCountryHouse }

function TCountryHouse.BuildChimney: string;
begin
  Result:=

  '              (   )                '#13#10 +
  '             (    )                '#13#10 +
  '              (    )               '#13#10 +
  '             (    )                '#13#10 +
  '               )  )                '#13#10 +
  '              (  (                 '#13#10 +
  '               (_)                 '#13#10 +
  '               [ ]                 '#13#10;
end;

function TCountryHouse.BuildRoof: string;
begin
  Result:=

  '       ___________________       '#13#10 +
  '      /\        ______    \      '#13#10 +
  '     //_\       \    /\    \     '#13#10 +
  '    //___\       \__/  \    \    '#13#10 +
  '   //_____\       \ |[]|     \   '#13#10 +
  '  //_______\       \|__|      \  '#13#10 +
  ' /XXXXXXXXXX\                  \ '#13#10 +
  '/_I_II  I__I_\__________________\'#13#10;
end;

function TCountryHouse.BuildWalls: string;
begin
  Result:=

  '  I_I|  I__I_____[]_|_[]_____I'#13#10 +
  '  I_II  I__I_____[]_|_[]_____I'#13#10 +
  '  I II__I  I     XXXXXXX     I'#13#10;
end;

function TCountryHouse.BuildFloor: string;
begin
  Result:=

  '~~~~~"   "~~~~~~~~~~~~~~~~~~~~~~~~  ';
end;

{ TCityHouse }

function TCityHouse.BuildChimney: string;
begin
  Result:=

  '                           ====  '#13#10 +
  '                           !!!!  '#13#10;
end;

function TCityHouse.BuildRoof: string;
begin
  Result:=

  '      ==========================      '#13#10 +
  '    %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%    '#13#10 +
  '  %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%  '#13#10 +
  '%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%'#13#10;
end;

function TCityHouse.BuildWalls: string;
begin
  Result:=

  '  ||      _____          _____    ||'#13#10 +
  '  ||      | | |          | | |    ||'#13#10 +
  '  ||      |-|-|          |-|-|    ||'#13#10 +
  '  ||      #####          #####    ||'#13#10 +
  '  ||                              ||'#13#10 +
  '  ||      _____   ____   _____    ||'#13#10 +
  '  ||      | | |   @@@@   | | |    ||'#13#10 +
  '  ||      |-|-|   @@@@   |-|-|    ||'#13#10 +
  '  ||      #####   @@*@   #####    ||'#13#10 +
  '  ||              @@@@            ||'#13#10;
end;

function TCityHouse.BuildFloor: string;
begin
  Result:=

  '******************____****************'#13#10 +
  '**************************************';
end;

end.CodeProject