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:
Some related reading below: