Let's recap this requirements from the user perspective:
Increasing the Health is a significant gameplay event In Breath of the Wild
(solving puzzles in shrines to get Spirit Orbs, then find and pray to a Goddess Statue),
and thus the increase is always 1 Heart each time, no more or less.
Now it makes sense to define a new unit in Health
that equals 4 Points,
that is visually represented as a Heart (or something else),
as the increase should always be in that unit.
I add a new domain definition Unit that maps to the visualized Heart
Domain Model | View / GUI |
---|---|
(Avatar) Health | Life Gauge |
Unit | Heart (holds 4 Fractions) |
Points | Heart Fractions |
With Unit defined, I realize that it's beneficial to redefine some 'points' variables into 'unit':
startingPoints
-> startingUnits
Units
, thus fits the Units
constraintFullPoints
, it makes sense to stay as pointsMaxNegativePointsForInstantKillProtection
-> MaxNegativeUnits...
Units
, thus fits the Units
constraintNow let's reword the requirements focusing on the domain model and using its updated terminology:
FullPoints
can be increased by 1 Unit
at a timeCurrentPoints
get same value as FullPoints
MaxUnits
(hereby named) is 30
The code shown below is focused on the current cycle step, thus only showing the relevant lines .
The complete code and the Unity project is on GitHub.
When doing a wholistic refactoring with a clear goal,
it might make more sense to start with the production code, as is the case now.
Health
I forsee that it will be beneficial to define a new constant PointsPerUnits
in Health
to make points <-> units conversation easy and readable.
I then rename the variable names and convert the values using PointsPerUnits
// Health.cs
public const int PointsPerUnit = 4;
public const int MaxNegativeUnitsForInstantKillProtection = -5;
public Health(int startingUnits)
{
ValidatePoints(startingUnits, 1);
FullPoints = CurrentPoints = startingUnits * PointsPerUnit;
}
// PointsPerUnit also used in InstantKillProtection calculation,
// which is not shown.
HealthTest
Now obviously many tests fail as the values need to converted to units.
I rename accordingly and convert the values, using PointsPerUnits
where appropriate.
// HealthTest.cs
[TestCase(3)]
[TestCase(1)]
public void CurrentPoints_HasStartingValue(int startingUnits)
{
var health = new Health(startingUnits);
Assert.That(health.CurrentPoints, Is.EqualTo(startingUnits * health.PointsPerUnit));
}
// other similar refactoring not shown.
Now all the tests pass again, and all is well in the universe.
IncreaseByUnit
The domain model focus will be a new method named IncreaseByUnit
.
FullPoints
) Focusing first on the FullPoints
property.
// HealthTest.cs
// inside nested class IncreaseByUnit
[Test]
public void FullPoints_Increase()
{
var health = new Health(3);
health.IncreaseByUnit();
Assert.That(health.FullPoints, Is.EqualTo(16));
}
I pass the test by using the defined constant PointsPerUnit
// Health.cs
public void IncreaseByUnit()
{
FullPoints += PointsPerUnit;
}
Now I focus on the other exit point, the CurrentPoints
property.
It ideally should be a new tests as it's about a different exit point
(see Good Unit Test definition in part 1).
CurrentPoints
)// HealthTest.cs
// inside nested class IncreaseByUnit
[Test]
public void CurrentPoints_Increase()
{
var health = new Health(3);
health.IncreaseByUnit();
Assert.That(health.CurrentPoints, Is.EqualTo(16));
}
// Health.cs
public void IncreaseByUnit()
{
FullPoints += PointsPerUnit;
CurrentPoints = FullPoints;
}
I simply add the line CurrentPoints = FullPoints
,
which is also the proper solution as the requirement is:
'CurrentPoints
get same value as FullPoints
(on increase)'.
All that is left is the maximum requirement.
The game will only give the user the option to increase the health
if the MaxUnits
has not been reached (and it costs 4 Spirit Orbs),
Thus the outside code should query Health
if maximum has been reached,
and if not, only then invoke IncreaseByUnit
.IncreaseByUnit
should though also handle improper usage in this regard,
and I'll add this handling at the end of this post.
The entry point will be a new property named IsMaxUnitsReached
.
// HealthTest.cs
// inside nested class IsMaxUnitsReached
[Test]
public void ReturnsTrue_WhenStartingUnitsAtMax()
{
int startingUnits = Health.MaxUnits;
var health = new Health(startingUnits);
Assert.That(health.IsMaxUnitsReached, Is.True);
}
[Test]
public void ReturnsFalse_WhenStartingUnitsNotAtMax()
{
int startingUnits = Health.MaxUnits - 1;
var health = new Health(startingUnits);
Assert.That(health.IsMaxUnitsReached, Is.False);
}
I added 2 test easily at the same time,
which should suffice to drive in the proper production implementation.
Here I can concise the test names
as the exit point is the same as the entry point,
thus I skip the exit point in the name (nested class already names it).
There are compile errors as bothMaxFullPoints
and IsMaxUnitsReached
are not defined.
// Health.cs
public const int MaxUnits = 30;
public bool IsMaxUnitsReached => MaxUnits == FullPoints / PointsPerUnit;
I define the constant and give it the proper value,
and I implement the read-only property as an Expression-Bodied Member.
IncreaseByUnit
If the outside code invokes IncreaseByUnit
when IsMaxUnitsReached
is true
,
the method should handle this improper usage.
// HealthTest.cs
// inside nested class IncreaseByUnit
[Test]
public void ThrowsError_WhenMaxUnitsReached()
{
int startingUnits = Health.MaxUnits;
var health = new Health(startingUnits);
var exception = Assert.Throws(Is.TypeOf<InvalidOperationException>(),
delegate
{
health.IncreaseByUnit();
});
Assert.That(exception.Message, Does.Match("invalid").IgnoreCase);
}
I deem I can use existing InvalidOperationException
from System
,
if I include an informative message for my case.
I assert that the message contains string invalid, ignoring case.
I'm intentionally not being more specific
to later minimize the chance the assert fails if the message is reworded.
This simple Resistance to refactoring makes this a better unit test.
(book Unit Testing Principles, Practices, and Patterns by Vladimir Khorikov,
lists Resistance to refactoring as one of the 4 pillars of good unit test).
// Health.cs
public void IncreaseByUnit()
{
if (IsMaxUnitsReached)
{
var message = $"Method invocation is invalid as {nameof(IsMaxUnitsReached)} is true";
throw new InvalidOperationException(message);
}
FullPoints += PointsPerUnit;
CurrentPoints = FullPoints;
}
I pass the test by adding the if condition at the start of the method.
Now the requirements hao been fulfilled and the Unity Test Runner looks like this: