TDD-ing Avatar Health in C# and C++

Part 4 - Taking Damage

The first Taking Damage requirement

Let's recap the first requirement under Taking Damage and Dying from the user perspective:

• Whenever Link takes damage he will lose a certain amount of Heart fractions
  equal to the damage points taken (always losing at least 1 fraction)

Let's reword this focusing on the domain model and using its terminology:

• Avatar will lose Health CurrentPoints equal to Damage Points taken (always losing at last 1 Health Point)

Take Damage cycling

RED

I start by writing the test.
The entry point will be a new method named TakeDamage,
thus I write a nested class with same name,
and within it I write the test case CurrentPointsDecrease
(according to my naming convention, explained in Part 3).

// HealthTest.cs
// inside nested class TakeDamage
[Test]
public void CurrentPoints_Decrease()
{
	var health = new Health(12);
	health.TakeDamage(1);
	Assert.That(health.CurrentPoints, Is.EqualTo(11));
}

The test fails as the TakeDamage method is not defined.

GREEN

// Health.cs
public int CurrentPoints { get; private set; }

public void TakeDamage(int damagePoints)
{
	CurrentPoints -= damagePoints;
}

I'm confident that this effortless implementation
fulfills the requirement for all valid inputs.
It's clean and thus does not need a REFACTOR step.
But invalid values can be passed in, so I'll do a cycle for handling those.

Handle invalid input values

RED

A minus value for damagePoints is clearly invalid but what about 0 ?
I conclude that 0 is also invalid,
as an outside 'damage calculator' logic should take care of this earlier
(thus TakeDamage won't be invoked).
This also makes the design streamlined as it ensures that
CurrentPoints will always decrease on TakeDamage call.

This will be a similar test as the existing ThrowsError_WhenStartingPointsIsInvalid
but now the entry point is method TakeDamage,
and thus it will be inside the nested class TakeDamage.

// HealthTest.cs
// inside nested class TakeDamage
[TestCase(0)]
[TestCase(-1)]
public void ThrowsError_WhenDamagePointsIsInvalid(int damagePoints)
{
	var health = new Health(12);
	var exception = Assert.Throws(Is.TypeOf<ArgumentOutOfRangeException>(),
		delegate
		{
			health.TakeDamage(damagePoints);
		});
	Assert.That(exception.Message, Does.Match("invalid").IgnoreCase);
}

The value I pass into the constructor has no significance to this test,
and thus is not a TestCase parameter.

GREEN

I make the test pass by
duplicating the exception throwing code from the constructor,
and then renaming the parameter to damagePoints
(knowing that I'll refactor this code smell right away in the next step).

// Health.cs
public Health(int startingPoints)
{
	const int lowestValidValue = 1;
	if (startingPoints < lowestValidValue)
	{
		var message = $"Value {startingPoints} is invalid, it should be equal or higher than {lowestValidValue}";
		throw new ArgumentOutOfRangeException(nameof(startingPoints), message);
	}

	CurrentPoints = startingPoints;
}

public void TakeDamage(int damagePoints)
{
	const int lowestValidValue = 1;
	if (damagePoints < lowestValidValue)
	{
		var message = $"Value {damagePoints} is invalid, it should be equal or higher than {lowestValidValue}";
		throw new ArgumentOutOfRangeException(nameof(damagePoints), message);
	}

	CurrentPoints -= damagePoints;
}

REFACTOR

I refactor the duplicated code smell away.

// Health.cs
public Health(int startingPoints)
{
	ValidatePoints(startingPoints, 1);
	CurrentPoints = startingPoints;
}

public void TakeDamage(int damagePoints)
{
	ValidatePoints(damagePoints, 1);
	CurrentPoints -= damagePoints;
}

private static void ValidatePoints(int points, int lowestValidValue)
{
	if (points < lowestValidValue)
	{
		var message = $"Value {points} is invalid, it should be equal or higher than {lowestValidValue}";
		throw new ArgumentOutOfRangeException(nameof(points), message);
	}
}

When exception is thrown,
the message is followed by the stack trace in the console,
and therefore it is sufficient that the param name will simply be points
as you can follow the trace to see which method called ValidatePoints.

Having lowestValidValue as sent in parameter for method ValidatePoints
makes the method easy to reuse,
if another case comes later with different lowestValidValue.

ReSharper hints that ValidatePoints method can be made static,
and I agree, as it doesn't hinder testability nor design (it's private).
Making it static can yield a small performance benefit.

I ignore ReSharper's hint to 'invert if statement' as the current format
is more readable IMO and the nested braces are only one-level deep.

I run all the tests (and also did as needed while refactoring) to verify everything works.
Now the Test Runner looks like this: