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.