While refactoring my code using Test Driven Development (TDD), should I keep making new test cases for the new refactored code I am writing?
This question is bases on the following TDD steps:
- Write just enough of a test for code to fail
- Write just enough code for the test to pass
- Refactor
My doubt is in the refactor step. Should new unit test cases be written for refactored code?
To illustrate that, I will give a simplified example:
Suppose I am making a RPG and I am making a HPContainer system that should do the following:
- Allow the player to lose HP.
- HP should not go below zero.
To answer that, I write the following tests:
[Test]
public void LoseHP_LosesHP_DecreasesCurrentHPByThatAmount()
{
int initialHP = 100;
HPContainer hpContainer= new HPContainer(initialHP);
hpContainer.Lose(5)
int currentHP = hpContainer.Current();
Assert.AreEqual(95, currentHP);
}
[Test]
public void LoseHP_LosesMoreThanCurrentHP_CurrentHPIsZero()
{
int initialHP = 100;
HPContainer hpContainer= new HPContainer(initialHP);
hpContainer.Lose(200)
int currentHP = hpContainer.Current();
Assert.AreEqual(0, currentHP);
}
To satisfy the requirements, I implement the following code:
public class HPContainer
{
private int currentHP = 0;
public void HPContainer(int initialHP)
{
this.currentHP = initialHP;
}
public int Current()
{
return this.currentHP;
}
public void Lose(int value)
{
this.currentHP -= value;
if (this.currentHP < 0)
this.currentHP = 0;
}
}
Good!
The tests are passing.
We did our job!
Now let's say the code grows and I want to refactor that code, and I decide that adding a Clamper class as following is a good solution.
public static class Clamper
{
public static int ClampToNonNegative(int value)
{
if(value < 0)
return 0;
return value;
}
}
And as a result, changing the HPContainer class:
public class HPContainer
{
private int currentHP = 0;
public void HPContainer(int initialHP)
{
this.currentHP = initialHP;
}
public int Current()
{
return this.currentHP;
}
public void Lose(int value)
{
this.currentHP = Clamper.ClampToNonNegative(this.currentHP - value);
}
}
The tests still pass, so we are sure we did not introduce a regression in our code.
But my question is:
Should unit tests be added to the class Clamper?
I see two opposing arguments:
Yes, tests should be added because we need to cover
Clamperfrom regression. It will ensure that ifClamperever needs to be changed that we can do that safely with test coverage.No,
Clamperis not part of the business logic, and is already covered by the test cases of HPContainer. Adding tests to it will only make unnecessary clutter and slow future refactoring.
What is the correct reasoning, following the TDD principles and good practices?
adding a Clamper class as following is a good solution.You don't add code when refactoring. You extract or move existing code into new components. If you do it this way, the existing tests still will pass and test the new component. At least the extracted code. Then, before adding new code to the new component, you first write the tests for it. – Laiv Nov 12 '19 at 07:20in case I refactor out new reusable components, should I add new unit tests for these componentsis a different question from the one I asked. For instance, the difference between "refactoring out" a reusable component and "refactoring internal behavior" can be relevant for my question (and in fact, it is being addressed in many of the answers given) while not being relevant to your proposed question rephrasing. – ianmandarini Nov 12 '19 at 09:20Clamperpublic. A small detail, yet it has huge consequences. By making it public, you change the public surface (API, interface or whatever else you may choose to call it). As such, this isn't then a refactor; it's introducing new public functionality. So you should write a test for that functionality before introducing that public class. But it shouldn't be public in the first place. Mark itinternaland it just becomes an implementation detail, ie a refactor and no new test is needed. – David Arno Nov 12 '19 at 11:25