End in sight
As a refresher, let's remind ourselves of the requirements of FizzBuzz
Write a method that takes a whole number and returns a text representation and has the following behavior for whole numbers above 0 (does not include 0). We don't have to add any bounds checking in this.
1. Return the string form of the number
* 1 as input returns "1"
2. If input is a multiple of 3 return "Fizz"
* 6 as input return "Fizz"
3. If input is a multiple of 5 return "Buzz"
* 10 as input return "Buzz"
4. If input is a multiple of 3 and 5 return "FizzBuzz"
* 30 as input return "FizzBuzz"
and we're technically still working on 3. If input is a multiple of 5 return "Buzz".
And let's refresh our flow chart for reference.
And now - The flow.
We should still consider that we are currently working on a requirement; sticking to the flowchart and not jumping ahead. The requirement we're on 3. If input is a multiple of 5 return "Buzz". we should be unable to think of a test that will fail. We've got a pretty definitive line in place. This will force us to select a new requirement, and there's only one left - 4. If input is a multiple of 3 and 5 return "FizzBuzz".
With that selected, we can add a comment of the intent we will write a test for. Pretty simple one is "Input of 15 returns FizzBuzz" - which we'll add as a comment. main 8eaf524
As this is the first test of the new requirement, and I've covered why I don't think a copy/paste approach is going to save time, fresh test it is! With our assert first main 120bbd6. Quickly followed by filling out the rest of the test main 7b8c3e2.
//Input of 15 returns FizzBuzz
[TestMethod]
public void Given15ReturnsFizzBuzz()
{
//ARRANGE
int sourceInput = 15;
string expected = "FizzBuzz";
//ACT
string actual = Transform(sourceInput);
//ASSERT
actual.Should().Be(expected);
}
With all the tests running we will see the new one fails on the assert. Maybe not for our expected reason. If asked, I probably would have said we'd get "15" back. Even doing this SO MANY TIMES... if I don't stop and consider what could happen, I'm likely to get it wrong. And if such a small, simple problem can cause incorrect assumptions, how risky is it in larger and more complex code bases?
Of course it's obvious why we get "Buzz"
back. It's for the exact reason we didn't do 15
as one of our inputs; because we'd be doing it here.
It is giving us a clear and informative reason the test is failing, and we're checking it - which is how we know I screwed up what the expected result would be.
Following the flow down; we're now going to add the condition for the input, and return the expected value. main 0c09da1
public string Transform(int source)
{
if (source == 15) return "FizzBuzz";
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return "Fizz";
return source.ToString();
}
And we're green!
With the flow chart nicely describing how I approach each step, I know I've talked about why I think about it, how I think about it; and got it put in so I can have a good guide helping me stay on track.
Only thing to refactor here is removing the human language intent. main 0f80284
We haven't added much new stuff. Our naming is aligned. ReSharper is telling us the variables can be const
, so let's do that. main 7bfffb9
Does it make sense to change the test like that when we know it's going to be deleted as we (expect to) refactor into a single test for the requirement?
Absolutely
This is our practice. We're practicing improving the code. Do it all the time, not just for the code you expect to keep. This helps build make our knee-jerk reaction to improve the code. Tool's telling us something we should do, and that we normally do - Do it. Will it get undone, or deleted? Maybe. But it's a few keystrokes to make the change. With ReSharper ALT+ENTER, ENTER
. All done.
With that refactor out of the way... time for out next time, with a current requirement, and adding any number of 15's to 15 will not give us 'FizzBuzz'. I bet it'd be 'Buzz'.
Let's add just a single 15 and see how that goes for our intent. 30 should return FizzBuzz
main aaef4b5
I'll continue committing step by step to show the changes, but feel free to just commit once the test runs and fails.
We'll continue our pattern of using copy and paste for our non-first test in the requirement. main d30aab3
We then change the interior of the method to match our intent main e597053.
Finally we change the method name to match our intent main 4528404.
To highlight it once again - This test never gave us a false positive. I have the IDE set up to run the tests when there is a change detected - Never got a false positive. It's absolutely because it couldn't compile the code; but there was never a wrongly passing test. It's why I highly value this technique, ensures the tests are always in a state representing their accuracy for the system.
With that change, we're looking at a red test failing for the reason we expected, "Buzz" was returned instead of "FizzBuzz" as we expected.
Getting to the Simplest Code section of the flow chart, we once again add the input condition and return the expected value. main d1f0f9c
public string Transform(int source)
{
if (source == 30) return "FizzBuzz";
if (source == 15) return "FizzBuzz";
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return "Fizz";
return source.ToString();
}
Getting us back to green!
I haven't gone on about triangulation too much. Do we see the pattern in the code? I still argue that we don't yet. We only have two examples in the code, for both patterns. Did you pick up on the pattern for the lines with the modulus operator?
What do we do with that pattern? If you remember my opening implementation, nothing. That's never the full answer, but for now, it's a pattern with two examples, not yet to our heuristic of three.
Refactor
An early refactor for each of these tests has been to remove the human language intent. No reason to change that. Be sure to clean up the whitespace around where it was. We can always catch it later, but it's best, as mentioned, to do it as part of the change that'd create it. main 24d874d
We're at the point, again, where we're starting to want our code to clearly communicate that we're dealing with multiples of 15. Let's expand these lines out
if (source == 30) return "FizzBuzz";
if (source == 15) return "FizzBuzz";
to
if (source == 2 * 15) return "FizzBuzz";
if (source == 1 * 15) return "FizzBuzz";
This is clearly expressing the intent of the code. main f17a3ea
It shows us very clearly that it's in alignment with the requirement.
... Is it?
Our requirement is 3 * 5, not 15. While some may be able to map that quickly... that's not what's being asked for; we could have clearer communication of our intent. We want to express it as best we can. We should split 15 into 3 * 5.
if (source == 2 * 3 * 5) return "FizzBuzz";
if (source == 1 * 3 * 5) return "FizzBuzz";
Personally, I'm not a fan of that look. What's the association? Why's 6 being multiplied by 5? Doesn't make a lot of sense. We have to further think about, and decipher, what the code is trying to tell us. If the code has to try and tell us - It's not expressing intent as well as we'd like.
Fortunately there's easy constructs that will help us express the more closely related, coupled, values. main 5d26e9d
if (source == 2 * ( 3 * 5 )) return "FizzBuzz";
if (source == 1 * ( 3 * 5 )) return "FizzBuzz";
This lets the code tell me what's associated. Clearly it's 1 and 2 multiplying a 3 * 5.
Minimize Elements
When we get to looking at the minimizing of elements, the paran's around the 3 * 5
are extra. Our tools may tell us that they're not required.
Which is great; our tools are trying to help.
Remember that the 4 rules of Simple Design are in priority order. We've added these parenthesis to improve the Expresses Intent rule, which is "more important". When we make a change to improve a higher priority rule, we definitely don't want to remove it. Other wise we're in an infinite loop of adding and removing parenthesis.
Let's give the same expansion treatment to the two tests we have so that there's similarity between tests and code. main 6a0146b
[TestMethod]
public void Given15ReturnsFizzBuzz()
{
//ARRANGE
const int sourceInput = 1 * (3 * 5);
const string expected = "FizzBuzz";
//ACT
string actual = Transform(sourceInput);
//ASSERT
actual.Should().Be(expected);
}
[TestMethod]
public void Given305ReturnsFizzBuzz()
{
//ARRANGE
const int sourceInput = 2 * (3 * 5);
const string expected = "FizzBuzz";
//ACT
string actual = Transform(sourceInput);
//ASSERT
actual.Should().Be(expected);
}
Without more to refactor, we can move onto the 3rd test of our last requirement.
Testing for the last time
Even though we went quickly through the first part of the flow chart a couple times already, let's make sure we are explicit in our practice and ensure we're doing all of the steps.
- Current Requirement? - Yes.
- Can write a failing test - Yes.
- 3 * 15 is 45, which will give us Buzz
We use that failing test idea to generate our human language intent of //Given 45 should return FizzBuzz
.
Looking at the code; I just noticed that I've made a type in Given305ReturnsFizzBuzz
. Since we're still green; and haven't made any commits for the new flow... I can pretend we're still refactoring. So... I'm gonna sneak in a commit to fix that name.
If I'd made the commit for the human language intent, I wouldn't refactor the name. I'd be in the process of writing a new test; it's best to not intermingle them.
I keep post-it notes close by to make reminders about things like this that I catch while working in the code, but am not at a point it's reasonable/safe for me to make the change.
I feel comfortable making the change now, so it shall be done. main d9c6306
Now to the new test
NOW we can add //Given 45 should return FizzBuzz
. main b61ac20
We'll copy and paste to get our test scaffolding in place main 7916013 followed by an update to the interior of the method main c41c611 finished by a change to the name main 522da85.
[TestMethod]
public void Given45ReturnsFizzBuzz()
{
//ARRANGE
const int sourceInput = 3 * (3 * 5);
const string expected = "FizzBuzz";
//ACT
string actual = Transform(sourceInput);
//ASSERT
actual.Should().Be(expected);
}
With those changes, all the tests run and we see our failire for the expected reason and an informative failure message.
With an existing method, we'll want to add the condition to match out test input and return the expected value of the test. main abc4557
public string Transform(int source)
{
if (source == 3 * (3 * 5)) return "FizzBuzz";
if (source == 2 * (3 * 5)) return "FizzBuzz";
if (source == 1 * (3 * 5)) return "FizzBuzz";
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return "Fizz";
return source.ToString();
}
and we see green!
Refactor
As we c an see the pattern now of having to be a multiple of 3 and 5, we know a short cut to check that, the mod operator. Or, as C# likes, Remainder Operator
.
public string Transform(int source)
{
if (0 == source % (3 * 5)) return "FizzBuzz";
if (source == 3 * (3 * 5)) return "FizzBuzz";
if (source == 2 * (3 * 5)) return "FizzBuzz";
if (source == 1 * (3 * 5)) return "FizzBuzz";
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return "Fizz";
return source.ToString();
}
As this is the last opportunity to bring it up again - New code code at the top of the method. I'm not doing this because it's where the % 15
needs to go, but because we want our test to pass. When the condition is as high up in the method as possible, we have the best chance to make the test pass. If any other tests happen to hit that condition as well, we'll know we broke them. We saw this when we originally did 3
returning "3"
. Our test broke when our new if statement caught that value and returned a different than expected result.
We can't always do THE TOP of the method... without a little tweaking... but the highest in the method we can allows us to be sure that the code for the test we're working on is being executed and not caught by some other functionality. We can get to green the fastest and see what our next steps might be.
A quick comment out of the generalized line main beb121f, keeps us green so we know it's safe to delete them main ea0eb6f.
This gives us our FizzBuzz method
public string Transform(int source)
{
if (0 == source % (3 * 5)) return "FizzBuzz";
if (0 == source % 5) return "Buzz";
if (0 == source % 3) return "Fizz";
return source.ToString();
}
We, of course, don't want to forget to remove the human language intent. main 9a8c9c8
It's easy to get caught up on implementation and code refactoring that other things slip by. It's important to review the broad code.
Pretend it's a new test
Since we're actually on a requirement, and we haven't confirmed we're done - Let's go back to the top and work through the first couple steps.
This is where we have a requirement, but can't think of any new tests that'll fail. Which puts us to the step that guides us to refactor the tests for the requirement.
We'll follow the pattern we've had for the other multiple combined tests. We want to be a little careful here as we use 1, 2, 3
for our multiples, not 4. While it'd work; it isn't an accurate representation of the tests we currently have, and are trying to replace.
**Uh Oh
I'm sure some of you have been screaming at me that I totally messed up the name of our multiple of 5 test. It says return Fizz.... It should say buzz.
Clearly I've not been paying enough attention to the names of the tests... Whoops. Once that is fixed main d667697 we can move onto creating the single test for our latest requirement. main 575e42a
[TestMethod]
public void GivenMultipleOf3And5ReturnsFizzBuzz()
{
//ARRANGE
const int multiplicand = 3 * 5;
const string expected = "FizzBuzz";
List<int> multiplierList = new() { 1, 2, 3 };
int randomIndex = Rand.Next(multiplierList.Count);
int multiplier = multiplierList.ElementAt(randomIndex);
int sourceInput = multiplier * multiplicand;
//ACT
string actual = Transform(sourceInput);
//ASSERT
actual.Should().Be(expected);
}
In the test name an interior we use some form of 3 * 5
. This is reflecting the actual requirement specification.
And we can now confidently delete the tests of specific numbers. main 3e0e007
Next Flow Step
The next step in the flow chart has been to select a new requirement... but we can't... Seems like we need a final update to the flow chart to indicate if there are no more requirements, we stop.
TDD My Way
And that's TDD My Way.
Thank you for coming along on the adventure!