Tuesday, July 21, 2015

Roman Numerals Coding Kata

This is one of the classic TDD coding katas. I've only done this a dozen or so times but after the first 2 or 3 attempts I found that I always ended up with exactly the same code. Follow my steps and see if you would make the same leaps at the same point. Let me know where you would deviate, where you see me making mistakes, or where you see slight improvements.   Lets get started:

First, lets create a test class and our first test. At this point we are tentatively designing our API. It may change, but we're at least working out how we are likely to call the function from real code. We already see a little bit of duplication here in the test, but we'll deal with that later if necessary.
import static org.junit.Assert.assertEquals;

import org.junit.Test;

public class RomanNumeralTest {
 @Test
 public void test() {
  assertEquals("1","I", RomanNumerals.arabic2Roman(1));
 }
Obviously running this code with jUnit will fail as we haven't created the class under test yet, but run it anyway and verify that the failure is what you expect. This is an important step that sometimes gets missed by beginners or even veterans who are rushing ahead. We want our failures to be expected and for the error to make it obvious what has failed and where.

Now go ahead and make the smallest step possible in order to make the test pass: create the required class and method.
public class RomanNumerals {
 public static String arabic2Roman(int arabic) {
  return "I";
 }
}
The smallest step does introduce some duplication again. That "I" literal is used in both the test and the code. We will want to keep our eye on that and remove it as soon as possible, but for now we don't know how. We need more tests to triangulate. Check that you are now green and we'll move onto the next test.
public class RomanNumeralTest {
 @Test
 public void test() {
  assertEquals("1","I", RomanNumerals.arabic2Roman(1));
  assertEquals("2","II", RomanNumerals.arabic2Roman(2));
 }
}
Again, check that we are red and we'll move onto making the test pass, noticing as we do that there is even more duplication in those test cases. They're just too similar looking, with repeated literals.

public class RomanNumerals {
 public static String arabic2Roman(int arabic) {
  if (arabic == 2) {
   return "II";
  }
  return "I";
 }
}
Again, the simplest step to make the tests pass appears to be cheating, but there is still not enough to make a decision on how to design the algorithm to remove the duplications. Lets press on with another test. Its at this point that the duplication in the tests gets too much for me and I refactor that out a little.
public class RomanNumeralTest {

 @Test
 public void test() {
  testConverting(1, "I");
  testConverting(2, "II");
  testConverting(3, "III");
 }

 private void testConverting(int arabic, String roman) {
  assertEquals("test " + arabic, roman, RomanNumerals.arabic2Roman(arabic));
 }
}
Run again to prove that we are failing on the test that we expect. Having refactored the test it is possible that you introduced a problem with one of the earlier tests so we need to ensure it is the new test that is actually failing, and failing with a useful error message. Once we're happy we write the code to make this test pass.
public class RomanNumerals {
 public static String arabic2Roman(int arabic) {
  if (arabic == 3) {
   return "III";
  }
  if (arabic == 2) {
   return "II";
  }
  return "I";
 }
}
At this point the duplication becomes obvious and while we are green we take the chance to refactor it out.
public class RomanNumerals {
 public static String arabic2Roman(int arabic) {
  StringBuilder roman = new StringBuilder();
  while (arabic >= 1) {
   roman.append("I");
   arabic--;
  }
  return roman.toString();
 }
}
Now we are getting somewhere, but the next couple of tests feel like they are adding duplication without it being immediately obvious how it can be removed. I
 @Test
 public void test() {
  testConverting(1, "I");
  testConverting(2, "II");
  testConverting(3, "III");
  testConverting(4, "IV");
  testConverting(5, "V");
 }

And the simplest code to pass the tests. The code for IV and V looks like the first attempts at the code for I and II so maybe there is some hidden duplication here. I also don't like returning from the middle of a method.
public class RomanNumerals {
 public static String arabic2Roman(int arabic) {
  StringBuilder roman = new StringBuilder();
  if (arabic == 5) {
   return "V";
  }
  if (arabic == 4) {
   return "IV";
  }
  while (arabic >= 1) {
   roman.append("I");
   arabic--;
  }
  return roman.toString();
 }
}
Once we add the test for 6 it starts to become apparent what the duplication is, and we begin to see what our final algorithm may be.
public class RomanNumerals {
 public static String arabic2Roman(int arabic) {
  StringBuilder roman = new StringBuilder();
  if (arabic >= 5) {
   roman.append("V");
   arabic -= 5;
  }
  if (arabic == 4) {
   return "IV";
  }
  while (arabic >= 1) {
   roman.append("I");
   arabic--;
  }
  return roman.toString();
 }
}
Since we are green we can take the opportunity to make the duplication completely obvious by rewriting the code for when (arabic == 4) to match the other stanzas, and then make the steps necessary to remove it. At this point I have seen some other blogs on the Roman Numerals Kata create 2 parallel arrays with the mapping from one to the other in them. However, that word "mapping" should make it obvious that what we actually have here is a map, and we should use the Java Map interface to show the relationship. We need a Map that can be sorted in reverse order by the arabic number key. Here is my final solution with the rest of the tests and mappings included. Obviously there are smaller steps and tests to be created before you get here, but apart from adding more tests and the necessary numbers to the TreeMap, the code and algorithm stays the same.
public class RomanNumeralTest {

 @Test
 public void test() {
  testConverting(1,"I");
  testConverting(2,"II");
  testConverting(3,"III");
  testConverting(4,"IV");
  testConverting(5,"V");
  testConverting(6,"VI");
  testConverting(7,"VII");
  testConverting(8,"VIII");
  testConverting(9,"IX");
  testConverting(10,"X");
  testConverting(11,"XI");
  testConverting(20,"XX");
  testConverting(50,"L");
  testConverting(1954,"MCMLIV");
  testConverting(1990,"MCMXC");
  testConverting(2014,"MMXIV");
 }

 private void testConverting(int arabic, String roman) {
  assertEquals("converting " + arabic, roman, RomanNumerals.arabic2Roman(arabic));
 }
}


public class RomanNumerals {
 static Map<Integer,String> arabic2roman = new TreeMap<Integer,String>(Collections.reverseOrder());
 static {
  arabic2roman.put(1, "I");
  arabic2roman.put(4, "IV");
  arabic2roman.put(5, "V");
  arabic2roman.put(9, "IX");
  arabic2roman.put(10, "X");
  arabic2roman.put(40, "XL");
  arabic2roman.put(50, "L");
  arabic2roman.put(90, "XC");
  arabic2roman.put(100, "C");
  arabic2roman.put(400, "CD");
  arabic2roman.put(500, "D");
  arabic2roman.put(900, "CM");
  arabic2roman.put(1000, "M");
 }

 public static String arabic2Roman(int arabic) {
  StringBuilder roman = new StringBuilder();
  for (Map.Entry<Integer,String> entry : arabic2roman.entrySet()) {
   while (arabic >= entry.getKey()) {
    roman.append(entry.getValue());
    arabic-=entry.getKey();
   }
  }
  
  return roman.toString();
 }
}