I end up with this

After my refactoring the code looks like this:

Public Function i2r(i As Integer) As String

Application.Volatile

i2r = ""

i2r = i2r + m(i, "L", 50)

i2r = i2r + m(i, "Xl", 40)

i2r = i2r + m(i, "X", 10)

i2r = i2r + m(i, "IX", 9)

i2r = i2r + m(i, "V", 5)

i2r = i2r + m(i, "IV", 4)

i2r = i2r + m(i, "I", 1)

End Function

Public Function m(ByRef i As Integer, ByVal RomanCharacter As String, ByVal IntegerValue As Integer) As String

While i >= IntegerValue

m = m + RomanCharacter

i = i - IntegerValue

Wend

End Function

And the tests all work afterwards.

  • Then I refactor the i2r() function.

Public Function i2r(i As Integer) As String

Application.Volatile

i2r = m(i, "L", 50) +

m(i, "XL", 40) +

m(i, "X", 10) +

m(i, "IX", 9) +

m(i, "V", 5) +

m(i, "IV", 4) +

m(i, "I", 1)

End Function

I’m confident that I understand how this function is supposed to work so I quickly add in the 90 = XC , 100 = C , 400= CD, 500 = D, 900 = CM and 1000 = M code, their test cases, and a healthy smattering of tests in between. When I get to 1000 = ‘M’ I realised that my laziness with the function name m() was very sloppy and I change it. The variable named i wasn’t very good either… but, easy enough to fix.

By this stage you are probably miles ahead of me. But here are my parting thoughts.