isbn-verifier: Tune too-long inputs to catch more incorrect algorithms

[nathanielknight]

This might be more detail than is needed, but it's something I noticed when completing the exercise.

The approached the problem by iterating over the first ten digits of the input and applying the algorithm in the description; any trailing digits were ignored. This passed all the test cases in the example. However, the test cases for ISBNs that were too long failed because the digits that they did happen to test were not a valid ISBN, and it was pretty easy to craft a string that was too long (and therefore not an ISBN) but passed the validator.

So, the proposal:

For exercises validating ISBN length ("too long isbn" and "too long isbn no dashes"), the first ten digits of the input should be a valid ISBN in order to catch solutions that ignore trailing digits.

As an extra twist, to catch folks approaching the problem by iterating in reverse, the test input could be eleven digits, the first ten and last ten of which are valid ISBNs (e.g: 35982150886).

[petertseng]

Yeah I think I listed in #993 (comment) some I might like to see:

• all 11 resulting digits (with a coefficient of 11 on the leftmost one, naturally)
• the 10 rightmost digits
• the 10 leftmost digits

would be fine to have them as three separate cases, unless there is some number that happens to meet all three conditions simultaneously

[nathanielknight]

Ah, so you did!

For example, 35982150886 satisfies the second two conditions. In my case, I would have ended up with a coefficient of -1 on the last digit. I wouldn't be surprised if an input could be crafted that would validate in all four cases without length checking, but I haven't found it yet.

[nathanielknight]

If there's no objection to this change, I'll can work on writing and validating a PR.

[petertseng]

I wouldn't be surprised if an input could be crafted that would validate in all four cases without length checking

Ah, it appears to not take a lot of effort to make an input for the first three. I was a little unsure about the fourth implementation mentioned, maybe I could get some help on it. I used some code

def isbn(digits)
  digits.each_with_index.sum { |c, i| (i + 1) * c } % 11 == 0
end

impls = [
  ->(ds) { isbn(ds.first(10)) },
  ->(ds) { isbn(ds.last(10)) },
  ->(ds) { isbn(ds) },
]

10.times {
  matching_impls = []
  digits = []
  until matching_impls.size == 3
    digits = Array.new(11) { rand(10) }
    matching_impls = impls.select { |f| f[digits] }
  end

  puts digits.join.reverse
}

So some numbers that appear to meet multiple conditions (maybe someone could check my work by hand to make sure I didn't make a mistake, I didn't check)

60289533775
98245726788
84795482314
41210348489
79514408411
57909897480
46705795921
87765289284
84637482879
85947603454

[petertseng]

I checked one number by hand, it seems fine

> 9 * 11 + 8 * 10 + 2 * 9 + 4 * 8 + 5 * 7 + 7 * 6 + 2 * 5 + 6 * 4 + 7 * 3 + 8 * 2 + 8
 => 385
> 8 * 10 + 2 * 9 + 4 * 8 + 5 * 7 + 7 * 6 + 2 * 5 + 6 * 4 + 7 * 3 + 8 * 2 + 8
 => 286
> 9 * 10 + 8 * 9 + 2 * 8 + 4 * 7 + 5 * 6 + 7 * 5 + 2 * 4 + 6 * 3 + 7 * 2 + 8
 => 319

[nathanielknight]

Those all work with my implementation as well 👍.

[petertseng]

The three incorrect implementations I described have been entered in https://github.com/petertseng/exercism-problem-specifications/blob/verify/exercises/isbn-verifier/verify.rb#L46-L69 . I want to put should_fail: true on all of them, but I cannot because two of them currently pass the tests in the JSON file. So the JSON file needs tests that the other two give incorrect answers on.

[petertseng]

The task here:

Craft input(s) that will be incorrectly accepted by one of three incorrect implementations. These incorrect implementations accept 11-digit ISBN-10s and do one of the following:

• validate all 11 resulting digits (with a coefficient of 11 on the leftmost one, naturally)
• validate the 10 rightmost digits
• validate the 10 leftmost digits

Some inputs that meet all three conditions are shown in #1199 (comment) ; feel free to use just one those, or use your own if you prefer.

Now that you have decided what input(s) to add:

• Add that/those input(s) to the JSON file.
• Increase second version component of the JSON file. If current version is x . y . z, make the new version x . y+1 . 0
• Use a word in https://help.github.com/articles/closing-issues-using-keywords/ in your commit message to indicate that you have closed this issue.
• Write in your commit message why this input is valuable to add. A summary of the discussion in this issue will suffice.