diff --git a/isbn.go b/isbn.go index e5b0e0b..b53f02a 100644 --- a/isbn.go +++ b/isbn.go @@ -12,10 +12,14 @@ func (i10 isbn10) Verify(code string) bool { } sum, multiply := 0, 10 - for _, n := range code { + for i, n := range code { var digit int switch { + // 'X' (value 10) is valid only as the check digit (final position). case n == 'X': + if i != len(code)-1 { + return false + } digit = 10 case isNotNumber(n): return false @@ -48,7 +52,9 @@ func (i10 *isbn10) Generate(seed string) (int, error) { multiply-- } - return 11 - sum%11, nil + // ISBN-10 check digit (ISO 2108): (11 - (sum mod 11)) mod 11, range 0-10. + // The outer mod 11 maps the sum%11==0 case to 0 instead of an out-of-range 11. + return (11 - sum%11) % 11, nil } // Verify implements checkdigit.Verifier interface. diff --git a/isbn_edge_test.go b/isbn_edge_test.go new file mode 100644 index 0000000..79046e7 --- /dev/null +++ b/isbn_edge_test.go @@ -0,0 +1,191 @@ +package checkdigit_test + +import ( + "fmt" + "testing" + + "github.com/osamingo/checkdigit" +) + +// TestIsbn10_Generate_Mod11Zero covers the sum%11==0 boundary where the +// check digit must be 0 (not the out-of-range 11 produced by the buggy +// `11 - sum%11` expression that missed the outer `% 11`). +func TestIsbn10_Generate_Mod11Zero(t *testing.T) { + t.Parallel() + + cases := map[string]struct { + in string + out int + }{ + "All zeros": { + in: "000000000", + out: 0, + }, + "Weighted sum divisible by 11": { + in: "055555555", + out: 0, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + r, err := checkdigit.NewISBN10().Generate(c.in) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if c.out != r { + t.Errorf("not equal, expected = %d, given = %d", c.out, r) + } + if r < 0 || r > 10 { + t.Errorf("check digit out of range [0,10]: %d", r) + } + }) + } +} + +// TestIsbn10_Generate_NonZeroRegression guards the non-zero check-digit +// cases (sum%11 != 0) that were already correct and must remain unchanged. +func TestIsbn10_Generate_NonZeroRegression(t *testing.T) { + t.Parallel() + + cases := map[string]struct { + in string + out int + }{ + "Check digit 9": { + in: "000000001", + out: 9, + }, + "Check digit 10 (X)": { + in: "000000006", + out: 10, + }, + "Arbitrary seed -> 10 (X)": { + in: "123456789", + out: 10, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + r, err := checkdigit.NewISBN10().Generate(c.in) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if c.out != r { + t.Errorf("not equal, expected = %d, given = %d", c.out, r) + } + }) + } +} + +// TestIsbn10_Verify_XWrongPosition covers the false-accept defect where +// 'X' (value 10) was accepted in any position. 'X' is valid only as the +// check digit (the final character) per ISO 2108. +func TestIsbn10_Verify_XWrongPosition(t *testing.T) { + t.Parallel() + + cases := map[string]struct { + in string + out bool + }{ + "X at first position": { + in: "X026515627", + out: false, + }, + "X at second position": { + in: "0X00000009", + out: false, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + ret := checkdigit.NewISBN10().Verify(c.in) + if c.out != ret { + t.Errorf("not equal, expected = %v, given = %v", c.out, ret) + } + }) + } +} + +// TestIsbn10_Verify_ValidXLastPosition confirms that a valid ISBN-10 with +// 'X' in the check-digit position is still accepted, plus basic +// valid/invalid controls. +func TestIsbn10_Verify_ValidXLastPosition(t *testing.T) { + t.Parallel() + + cases := map[string]struct { + in string + out bool + }{ + "Valid X in last position": { + in: "000000006X", + out: true, + }, + "Valid all zeros": { + in: "0000000000", + out: true, + }, + "Invalid check digit": { + in: "0000000001", + out: false, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + ret := checkdigit.NewISBN10().Verify(c.in) + if c.out != ret { + t.Errorf("not equal, expected = %v, given = %v", c.out, ret) + } + }) + } +} + +// TestIsbn10_RoundTrip checks the Generate -> Verify round-trip property +// over a range of seeds, including the sum%11==0 boundary cases. +func TestIsbn10_RoundTrip(t *testing.T) { + t.Parallel() + + prov := checkdigit.NewISBN10() + + seeds := []string{ + "000000000", // sum%11==0 -> check digit 0 + "055555555", // sum%11==0 -> check digit 0 + "000000001", // -> 9 + "000000006", // -> 10 (X) + "123456789", // -> 10 (X) + "002651562", // -> 8 + "007231592", // -> 10 (X) + "155860832", // -> 10 (X) + } + + for _, seed := range seeds { + t.Run(seed, func(t *testing.T) { + t.Parallel() + + cd, err := prov.Generate(seed) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + var code string + if cd == 10 { + code = seed + "X" + } else { + code = fmt.Sprintf("%s%d", seed, cd) + } + if !prov.Verify(code) { + t.Errorf("round-trip failed: Generate(%q)=%d -> Verify(%q)=false", seed, cd, code) + } + }) + } +}