IMO the current tests for this exercise do not match the description in instructions.md
:
If the input is the correct size, but not recognizable, your program should return ‘?’
If the input is the incorrect size, your program should return an error.
However, the relevant tests look like this:
test(
"Input with a number of lines that is not a multiple of four raises an error") {
pending
OcrNumbers.convert(List(" _ ",
"| |",
" ")) should be("?")
}
test(
"Input with a number of columns that is not a multiple of three raises an error") {
pending
OcrNumbers.convert(List(" ",
" |",
" |",
" ")) should be("?")
This feels doubly wrong because the tests neither test for returning an error (as indicated in the instructions) nor do they test for raising an error (as per the test descriptions).
So this PR changes all tests to use Optional[String]
as the return type, with None
being asserted for the two tests listed above.
I also think we should add two additional tests, both List()
and List("", "", "", "")
should IMO return None
, not “?”, so I added those in a second commit.
I understand that this will make all existing solutions fail the test suite when they re-run, but I think it’s worth it. It’s also more consistent with other exercises, which use optionals in a similar way (e.g. Grains, Hamming Distance, Largest Series Product, Queen Attack.)
Link to auto-closed PR: Change `OcrNumbersTest` to use optionals by citizen428 · Pull Request #848 · exercism/scala · GitHub