[numerals] - ConvertToArabic solution

[raphaelbrugier]

First, let me tell you I really enjoyed this book and learned a ton about Go! I'd like to contribute another solution for the converToArabic method in the numeral chapter.

I found the current solution too complicated to follow. At this point, TDD has already helped to write and refactor the previous function. The interesting point to me is to reuse the test table to write the second function, the algorithm is pretty much the same than the previous function.

Here is the solution I wrote before:

func ConvertToArabic(roman string) uint16 {
    var res uint16 = 0

    for len(roman) > 0 {
        for _, numeral := range allRomanNumerals {
            if strings.HasPrefix(roman, numeral.Symbol) {
                res += numeral.Value
                roman = strings.TrimPrefix(roman,numeral.Symbol)
            }
        }
    }

    return res
}

What I like is that it forced me to look at the strings library.

Lmk what you think.

[nij4t]

I have benchmarked both original and my implementations and these are the results:

original:

Running tool: /home/nij4t/sdk/go1.15.6/bin/go test -benchmem -run=^$ -coverprofile=/tmp/vscode-govI5zVd/go-code-cover -bench .

goos: linux
goarch: amd64
BenchmarkConvertToRoman-8         268189          5909 ns/op         312 B/op         37 allocs/op
BenchmarkConvertToArabic-8         28428         50801 ns/op        6360 B/op        212 allocs/op
PASS
coverage: 96.4% of statements
ok      _/home/nij4t/workspace/github.com/Learing-Go-with-tests/roman-numerals  3.525s

mine:

Running tool: /home/nij4t/sdk/go1.15.6/bin/go test -benchmem -run=^$ -coverprofile=/tmp/vscode-govI5zVd/go-code-cover -bench .

goos: linux
goarch: amd64
BenchmarkConvertToRoman-8         252447          4582 ns/op         312 B/op         37 allocs/op
BenchmarkConvertToArabic-8        198014          6419 ns/op           0 B/op          0 allocs/op
PASS
coverage: 100.0% of statements
ok      _/home/nij4t/workspace/github.com/Learing-Go-with-tests/roman-numerals  2.551s

mine implementation:

func ConvertToArabic(roman string) uint16 {
    var result uint16

    for _, numeral := range RomanNumeralsTable {
        for roman != "" {
            if strings.HasPrefix(roman, numeral.Symbol) {
                result += numeral.Value
                roman = roman[len(numeral.Symbol):]
            } else {
                break
            }
        }
    }

    return result
}

Looks similar to the example above, though I haven't used the trim function which, I assume, may slow down the execution. I also check the roman to be an empty string as it will end up as an empty string eventually.

[nickycakes]

I honestly anticipated a solution like those above when going into the 2nd half of this chapter, as it makes logical sense working the arabic -> roman conversion backwards. following the example in the chapter was confusing and took a lot of time to get through on the way to the main point of the chapter which is the property based tests

[hkennyv]

+1 I arrived to a similar solution to this as well and found the one in the book a bit complicated. IMO for the purposes of the book, I'd prefer to see something simpler and easier to explain (regardless of performance).

I know the focus is on demonstrating the iteration, so if there is a way to arrive at this kind of solution iteratively, I'd be in favor of that.