Before last week, I attended Roy Osherove’s TDD Masterclass, I do not think I would have had the “you know what’s” to be able to put up an alternative solution to one of Uncle Bobs (aka Robert Martin) examples of doing Test Driven Development, but here I am publishing the the solution I arrived at and suggesting that my solution better servers the values of Readability, Maintainability and trustworthiness.
In Uncle Bobs defence his post is dated Apr 2006 and the art of Test Driven Development has moved on leaps and bounds in the past few years.
The Problem
Write a class named “PrimeFactors” that has one static method: generate.
The generate method takes an integer argument and returns a List<Integer>. That list contains the prime factors in numerical sequence.
To see the details and how it was previously solved.
My Solution
Although the original is coded in Java and mine in C#, there is enough similarities for you to be able to understand both – weird that really 
The Tests
1: using System.Collections.Generic;
2: using NUnit.Framework;
3:
4: namespace Product.Tests
5: {
6: [TestFixture]
7: public class CalculatorTests
8: {
9: private Calculator c;
10:
11: [SetUp]
12: public void SetUp()
13: {
14: c = new Calculator();
15: }
16:
17: [Test]
18: public void Generate_Zero_ReturnEmptyList()
19: {
20: List<int> result = c.Generate(0);
21:
22: Assert.AreEqual(0, result.Count);
23: }
24:
25: [Test]
26: public void Generate_One_ReturnEmptyList()
27: {
28: List<int> result = c.Generate(1);
29:
30: Assert.AreEqual(0, result.Count);
31: }
32:
33: [TestCase(2)]
34: [TestCase(3)]
35: [TestCase(5)]
36: public void Generate_PrimeNumber_ReturnPrimeNumber(int expected)
37: {
38: List<int> result = c.Generate(expected);
39:
40: Assert.AreEqual(expected, result[0]);
41: }
42:
43: [TestCase(4,2,2)]
44: [TestCase(6,2,3)]
45: public void Generate_NonPrimeNumberDivisableByTwo_ReturnTwoProducts(int number, int firstPrime, int secondPrime)
46: {
47: List<int> result = c.Generate(number);
48:
49: Assert.AreEqual(firstPrime, result[0]);
50: Assert.AreEqual(secondPrime, result[1]);
51: }
52:
53: [Test]
54: public void Generate_NonPrimeNumberWithThreeProducts_ReturnThreeProducts()
55: {
56: List<int> result = c.Generate(8);
57:
58: Assert.AreEqual(2, result[0]);
59: Assert.AreEqual(2, result[1]);
60: Assert.AreEqual(2, result[2]);
61: }
62:
63: [Test]
64: public void Generate_NonPrimeNumberNotDivisableByTwoWithTwoProducts_ReturnProducts()
65: {
66: List<int> result = c.Generate(9);
67:
68: Assert.AreEqual(3, result[0]);
69: Assert.AreEqual(3, result[0]);
70: }
71: }
72: }
The Production Code
1: using System.Collections.Generic;
2:
3: namespace Product
4: {
5: public class Calculator
6: {
7: private const int SMALLEST_PRIME = 2;
8:
9: public List<int> Generate(int i)
10: {
11: List<int> primes = new List<int>();
12:
13: int divider = SMALLEST_PRIME;
14: while (HasPrimes(i))
15: {
16: while (IsDivisable(i, divider))
17: {
18: i = AddPrimeToProductsAndReduce(i, primes, divider);
19: }
20: divider++;
21: }
22: return primes;
23: }
24:
25: private bool IsDivisable(int i, int divider)
26: {
27: return i%divider == 0;
28: }
29:
30: private bool HasPrimes(int i)
31: {
32: return i >= SMALLEST_PRIME;
33: }
34:
35: private int AddPrimeToProductsAndReduce(int i, List<int> primes, int prime)
36: {
37: primes.Add(prime);
38: i /= prime;
39: return i;
40: }
41: }
42: }
I understand that showing the final solution does not prove the value of TDD, I will be posting a web cast of this process to really show how TDD helps you code to evolve and also how I adhere to the Red, Green, Refactor steps to achieve readable, maintable and trustworthy code.
Where do I think mine is better?
The Tests
The tests all adopt the naming convention Method_Scenario_Behaviour
The Act and Assert are kept separate for readability.
I have not used any logic in my tests, unlike the list()method used in Uncle Bobs.
The Production Code
No magic numbers, I have extracted well named constants.
Refactored the logic in ther generate method into more readable methods. For example line 14.
Is much easier to understand than the line
1: for (int candidate = 2; n > 1; candidate++)
I don’t like the way the while loops were refactored into for loops to reduce the code. If you look closely at the for loop above you will see that it does not follow the normal convention where the evaluation part is related to the incremental part. This I feel makes it easy to misread and hard to read if you don’t miss it.
Overall I think that the my code, although longer than Uncle Bobs is much easier to read and therefore easier to maintain.
I would love to hear feedback on the code, perhaps I have a couple of refactoring’s that could improve it still further.
