isbn-verifier: adding test case for 0 < length < 10

[mikestratton]

fixes #1216

[rpottsoh]

👍 Thanks @mikestratton!

[petertseng]

Thanks for your interest!

Before I can call the issue closed, I mentioned at least two incorrect implementations in the issue.

These two incorrect implementations already reject 134456729, therefore they still pass all tests in the test suite even though they should fail. I did the math on what these two implementations calculate (I did this by hand, so feel free to point it out if I made a mistake)

  1 * 9
+ 3 * 8
+ 4 * 7
+ 4 * 6
+ 5 * 5
+ 6 * 4
+ 7 * 3
+ 2 * 2
+ 9 * 1
= 168

and 168 is not divisible by 11

  1 * 10
+ 3 * 9
+ 4 * 8
+ 4 * 7
+ 5 * 6
+ 6 * 5
+ 7 * 4
+ 2 * 3
+ 9 * 2
= 209

and 209 is also not divisible by 11. I was obviously incorrect on that, this is what you get when you try to determine divisibility in your head.

I'd like to ask for test cases that these two implementations would incorrectly accept.

I'll leave it up to choice whether this should be one test case covering both, or one test case for each. The proposed input 134456729 that just has 9 characters but is already correctly handled can either stay or be deleted, also by your choice.

[mikestratton]

209/11 = 19

[petertseng]

My mistake. Let me see why the implementation isn't is rejecting the case.

My mistake. Let me see why the implementation isn't rejecting the case.

[petertseng]

yeah my mistake; the automated check was itself faulty. (I had written apppend which was obviously incorrect, therefore my implementation rejected but only because the method name didn't exist)

Okay. I'd like to merge this.

For a second case that the other implementation will incorrectly accept, I'm happy to simply open another issue and decrease the scope of the existing issue.

Or, if you would like to do it in this same PR, you are free to.

[petertseng]

When merging, I plan to add a few details from #1216 into the commit message. It is better to have them in the commit message than make someone have to open GitHub to see what #1216 is.

[petertseng]

We have two approvals, and the linked issue has been open long enough that if there were any objections, people have had enough time to raise them in the issue. So I think we should merge it, without needing to wait longer.

I have a path forward for the remaining issue I want to tackle (just make a new issue about the other implementation), so I'm just going with that plan.

Thanks!

[petertseng]

indentation interesting, but since we are not enforcing it automatically, my only argument is "it should be the same so that it is consistent, which will maybe make it easier to read"