delphiXtreme

This blog post brought to you by the wonderful ambiguity of the English language!

And now back your regularly scheduled program ….

“How do I/Should I test private methods?”

This question seems to come up quite often. In this article I hope to convince that not only do you not need to test your private methods but that it is also bad practice.

Do what do I mean when I say “don’t test your private methods”? Am I telling you not to test code that might break? Not at all. What I mean is when you follow these two principles:

1. Code from the outside-in
2. Code to an interface, not an implementation

you will not need to go to any lengths to test your private methods. The first principle is why you won’t need to test private methods and the second is why it’s bad practice to test them. To better understand this lets look an example:

We’ll start with a test

procedure MyStack.ShouldPushItems;
begin
  FStack.Push('foo');
  CheckEquals(1, FStack.GetCount);
end;

And now lets fill in some code to make it compile:

type
  TMyStack = class(TObject)
  private
    FData : TStringList;
    procedure DoPush(const Value : string);
  public
    constructor Create;
    destructor Destroy; override;
    procedure Push(const Value : string);
    function GetCount : Integer;
  end;

implementation

procedure TMyStack.DoPush(const Value : string);
begin
  FData.Insert(0, Value);
end;

procedure TMyStack.Push(const Value : string);
begin
  DoPush(Value);
end;

function TMyStack.GetCount : Integer;
begin
  Result := FData.Count;
end;

No surprises here (except that I left out the constructor and destructor code). The test now passes with this code. Now how do we make sure that the value we pushed onto the stack was actually stored? That is to say, how do we know DoPush() did its job? How do we test DoPush()? We can’t look at the internal storage because it’s private and we don’t want to break encapsulation for testing. We don’t yet have a Pop() method as our test didn’t call for it.

So what do we do? Right now our testing context is very generalized and non-specific; just a stack. Let see what changing our testing context could get us:

procedure MyStackWithOneItem.SetUp;
begin
  FStack := TMyStack.Create;
  FStack.Push('foo');
end;

procedure MyStackWithOneItem.ShouldHaveACountOfOne;
begin
  CheckEquals(1, FStack.GetCount);
end;

procedure MyStackWithOneItem.ShouldPopThePushedItem;
begin
  CheckEquals('foo', FStack.Pop);
end;

And now some code to make it compile:

{ this is a private method }
function TMyStack.DoPop : string;
begin
  Result := FData[0];
  FData.Delete(0);
end;

function TMyStack.Pop : string;
begin
  Result := DoPop;
end;

So now our testing context is a stack with one item pushed onto it. We already know that ShouldHaveACountOfOne() will pass since we proved it in the previous iteration of the code. However, we still need to prove that the item we pushed onto the stack was indeed stored. That’s where ShouldPopThePushedItem() comes in. We’re able to infer that DoPush() did its job by verifying it using Pop().

But wait! What about DoPop()! We didn’t test that! You right … while we did check that the value returned from Pop() matched our pushed item there is one behavior of DoPop() that we didn’t check: the stack should now be empty. Let add a test:

procedure MyStackWithOneItem.ShouldBeEmptyAfterPop;
begin
  FStack.Pop;
  CheckEquals(0, FStack.GetCount);
end;

Remember that DUnit runs SetUp() before each test. ShouldBeEmptyAfterPop() will find the stack containing one item even though it executes after ShouldPopThePushedItem().

By changing our testing context we were able to verify that our private methods worked the way we expected them to.

Now, why is it bad practice to test private methods? Well lets say we made some decisions that led us to expose the internal data storage so that we could verify that Push() worked by accessing the internal storage. What happens when we refactor the code to use some other form of storage? This is where “Code to an interface, not an implementation” comes in. In our case Push(), Pop() and GetCount() make up the interface to the stacks implementation. Our test shouldn’t care about how the stack does its work any more than our customers care. As long as Push(), Pop() and GetCount() work the way they’re expected to work then all is well.

Given this arrangement let say that we decide to refactor the stack to store its data in database. I’ll leave the code for that as an exercise for the reader (!). Even though this is a large change to the code we can proceed with confidence. Why? Because our tests will tell us when we’ve broken something. Why can we rely on the tests? Because when we embarked on this big change the tests were passing and as we make this change, we don’t have to change the tests.

So, please, don’t test your privates. =)

BDD, XP