Skip to content

Commit

Permalink
fixing some inconsistencies that brought about some confusion for qui…
Browse files Browse the repository at this point in the history
  • Loading branch information
quii committed Jan 30, 2020
1 parent 528fe9f commit e18a9d4
Showing 1 changed file with 11 additions and 11 deletions.
22 changes: 11 additions & 11 deletions roman-numerals.md
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ type RomanNumeral struct {
Symbol string
}

var RomanNumerals = []RomanNumeral {
var allRomanNumerals = []RomanNumeral {
{10, "X"},
{9, "IX"},
{5, "V"},
Expand All @@ -423,7 +423,7 @@ func ConvertToRoman(arabic int) string {

var result strings.Builder

for _, numeral := range RomanNumerals {
for _, numeral := range allRomanNumerals {
for arabic >= numeral.Value {
result.WriteString(numeral.Symbol)
arabic -= numeral.Value
Expand Down Expand Up @@ -516,10 +516,10 @@ func TestRomanNumerals(t *testing.T) {
- I removed `description` as I felt the _data_ described enough of the information.
- I added a few other edge cases I found just to give me a little more confidence. With table based tests this is very cheap to do.

I didn't change the algorithm, all I had to do was update the `RomanNumerals` array.
I didn't change the algorithm, all I had to do was update the `allRomanNumerals` array.

```go
var RomanNumerals = []RomanNumeral{
var allRomanNumerals = []RomanNumeral{
{1000, "M"},
{900, "CM"},
{500, "D"},
Expand Down Expand Up @@ -657,7 +657,7 @@ func ConvertToArabic(roman string) int {
potentialNumber := string([]byte{symbol, nextSymbol})

// get the value of the two character string
value := romanNumerals.ValueOf(potentialNumber)
value := allRomanNumerals.ValueOf(potentialNumber)

if value != 0 {
total += value
Expand Down Expand Up @@ -698,7 +698,7 @@ func ConvertToArabic(roman string) int {
potentialNumber := string([]byte{symbol, nextSymbol})

// get the value of the two character string
value := romanNumerals.ValueOf(potentialNumber)
value := allRomanNumerals.ValueOf(potentialNumber)

if value != 0 {
total += value
Expand Down Expand Up @@ -746,14 +746,14 @@ func ConvertToArabic(roman string) int {
// build the two character string
potentialNumber := string([]byte{symbol, nextSymbol})

if value := romanNumerals.ValueOf(potentialNumber); value != 0 {
if value := allRomanNumerals.ValueOf(potentialNumber); value != 0 {
total += value
i++ // move past this character too for the next loop
} else {
total++ // this is fishy...
}
} else {
total+=romanNumerals.ValueOf(string([]byte{symbol}))
total+=allRomanNumerals.ValueOf(string([]byte{symbol}))
}
}
return total
Expand Down Expand Up @@ -787,14 +787,14 @@ func ConvertToArabic(roman string) int {
symbol := roman[i]

if couldBeSubtractive(i, symbol, roman) {
if value := romanNumerals.ValueOf(symbol, roman[i+1]); value != 0 {
if value := allRomanNumerals.ValueOf(symbol, roman[i+1]); value != 0 {
total += value
i++ // move past this character too for the next loop
} else {
total++ // this is fishy...
}
} else {
total+=romanNumerals.ValueOf(symbol)
total+=allRomanNumerals.ValueOf(symbol)
}
}
return total
Expand Down Expand Up @@ -835,7 +835,7 @@ total++ // this is fishy...
We should never be just incrementing `total` as that implies every symbol is a `I`. Replace it with:

```go
total += romanNumerals.ValueOf(symbol)
total += allRomanNumerals.ValueOf(symbol)
```

And all the tests pass! Now that we have fully working software we can indulge ourselves in some refactoring, with confidence.
Expand Down

0 comments on commit e18a9d4

Please sign in to comment.