bowling: should we be precise about which roll, specifically, is erroneous?

[petertseng]

Take, for example, the sequence of rolls [5, 6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

In this, we would expect the 6 is the erroneous roll. By this, we mean that if game = roll 6 (roll 5 bowlingStart), no matter what you do to game you cannot get a score out of it.

The tests don't test that precisely - they currently cannot, since roll returns a Bowling, and there is no way to determine the above-described property of a Bowling without trying to roll it to completion.

So my question is: Would we like to be more precise about which roll is erroneous?

(I have argued the same in the Rust implementation)

The gist is that I like to fail fast and close to the source of the error, so that it is easy to understand what caused the error.

If we find this desirable, some proposals to do it:

• Add isValid :: Bowling -> Bool. I find this a little unclean.
• Change roll's type to roll :: Bowling -> Int -> Either ___ Bowling (the ___ can be any user-defined error they like; the tests only need to test isLeft on it)

And a final question:

Should this be brought to x-common? Note that there will be a slight challenge in how to represent the "this specific roll should fail" in JSON (my current leading thought is keeping the rolls array but then adding a roll_should_fail entry)

[rbasso]

Disclaimer: I didn't solve this exercises yet in any language.

I don't like to much the idea of having an isValid function, but you proposal to change the roll type seems sound, @petertseng.

If you want to be flexible in the type of error, maybe it can be generalized to any MonadError m e, but I never tried to write a test suite like this one before.

I would start with the rigid type Int -> Either String Bowling, because we'll need to supply a signature anyway, but making it more general is always nice. 😄

The only problem that I see in this approach is that we may be restricting too much the way to calculate the final score. Considering that we don't score partial games, is it hard to justify an interface that forces the student to partially evaluate it.

I'm divided in this one. The way it is now, it is impossible to know where it fails; changing the interface we may loose diversity.

[rbasso]

My bad. The tests already expect roll to be a function to fold rolls, so my argument makes no sense at all. 😁.

Considering that, I support your proposal, @petertseng.

[rbasso]

I imagine that you are also changing score, right?

[rbasso]

I had a hard time trying to discover why I had the feeling that something was strange in this exercise!

Now that I had some more time to think about it, I'll try to make a proper comment. 😄

About x-common

In x-comon there is something about a method roll that "should accept a single integer" and another one called score, that "should return the game's final score". Clearly, it was written with OOP in mind, and that can be confirmed just by looking at the title of exercism/problem-specifications#73.

Because of that, instead of not testing for intermediary functions, this object-oriented specification seems to promote testing just for intermediary functions. A more direct specification would probably only ask for a score function, to be applied directly to rolls.

So, considering what was discussed in exercism/discussions#41 and exercism/discussions#44, I think that the specification is over-prescriptive and deserves to be changed. Of course, I'm saying this from the perspective of a Haskell enthusiast.

About our current implementation

It is hard to transform object-oriented programming in functional programming, but I would risk saying that @abo64's implementation of bowling was correct and a faithful translation of what we have in x-common.

That said, it unavoidably suffers a little from over-prescriptiveness, as the source.

About the proposed changes

I consider your proposal to change the signature of roll a good improvement, @petertseng, so I support the change.

But before following that path, I would like to question if we should keep testing roll or diverge from x-common.

An alternative

What do you guys think about these signatures?

score :: [Int] -> Maybe Int
score :: [Int] -> Either String Int

We could take @petertseng's idea of ignoring the contents when it is a Left. Of course, we could also demand a somewhat more elaborated error handling:

data BowlingError = IncompleteGame
                  | InvalidRoll (Int, Int)

score :: [Int] -> Either BowlingError Int

[abo64]

Actually the Scala version uses Either, but only a wrapper Error around a String. And the error is never checked in the tests, so I thought a simple Maybe would be enough.
Having said that: @rbasso , your BowlingError suggestion (and then of course checking the Left properly in the tests) looks most convincing to me. But we should clearly state what the two InvalidRow parameters mean (perhaps with record notation?).

[abo64]

ok, then I will implement these changes within the next few days.

[rbasso]

I guess it would be nice to wait a little until @petertseng says what he thinks about it.

[petertseng]

Don't wait for me too long while I still have limited access (but in this case y'all got lucky)

So I think score :: [Int] -> Either BowlingError Int was the prevailing suggestion? This looks good. Can you clarify what the two InvalidRoll parameters are? The index of the invalid roll, and then the roll itself? That seems good (anything that at least indicates which roll is invalid will be good, you won't need to wait for me)

[abo64]

sorry @petertseng I was too quick here. Something like

data BowlingError = IncompleteGame
                  | InvalidRoll { rollIndex :: Int, rollValue :: Int }

?

[petertseng]

Seems good if that's what you were thinking. Happy to defer to @rbasso if has a different idea, but it was my best guess from seeing two ints.

[abo64]

And it was my best guess, too. :-)

[rbasso]

I'm fine with either, ignoring the Left content or demanding invalid roll's index and value.

So everybody agrees with diverging from x-common?!

[petertseng]

So everybody agrees with diverging from x-common?!

Surprisingly, I do. I think the diversion is reasonable and is better suited for our track.

And it is not so bad - we haven't diverged in the actual test cases, only how they are tested (omitting a roll function).

Any records of past diversions? I see one in crypto-square.

But there is an interesting point - the README can now be confusing for students if we only test the score. Shall we document the diversion in the HINTS.md to lessen the confusion?

And/or: Shall we propose making the x-common README less prescriptive?

[rbasso]

• HINTS.md 👍
• Data type in the stub solution 👍
• Open issue in x-common 👍

In x-common, I guess that removing the section Requirements in description.md would be enough to avoid confusion.