Error in implementation of Mod 11 algorithm

[martinjoiner]

Hi @komakino,

I have been using this library for a batch processing operation. It's a bit of a personal coding challenge where I write the same logic in 4 different languages.

My logic runs against a set of ~6000 real NHS numbers from the production data, checking that they are valid. This uses the factors [2,3,4,5,6,7,8,9,10].

However, when I ran the scripts and compared results, I noticed that your library reports significantly fewer valid checksums than the library I am using for the Ruby version of the logic. Your script reports ~5000 valid numbers whereas the Ruby library reports ~5500 (10% more).

The main difference I can see between the 2 libraries is right at the end of the calculation process where they both use switch statements but with 1 significant difference....

This library https://github.com/komakino/modulus11/blob/master/src/Modulus11.php#L24 is like this...

$calculus = 11 - $sum % 11;

switch($calculus){
    case 10: return 'X';
    default: return $calculus;
}

@badmanski's library https://github.com/badmanski/mod11/blob/master/lib/mod11.rb#L25 is like this...

remainder = sum % 11

case remainder
when 0 then remainder
when 1 then nil
else 11 - remainder
end

I am not 100% confident whose code is "correct" here but I think the key difference is that your version has the ability to return 11, whereas @badmanski's code does not.

This applies in the test case "648180980" where the total sum of each digit multiplied by it's factor is 275, which is a multiple of 11. This means the remainder when divided by 11 is 0 which when subtracted from 11 is 11. I believe the correct Mod11 checksum in this case should be 0 and my list of real NHS numbers appears to validate that.

Why do I care?

Your package is the top result on packagist.org in a search for the phrase "modulus 11" with 162,000+ downloads. It's popularity was the reason I chose your library. However if what I have discovered is a bug, then this could be seriously affecting many projects, a lot of which I suspect will be trusting it when validating NHS Numbers.

I notice @Jontsa has already raised a PR #1 to fix this back in Nov 2018 but you have not responded.

I am not sure what your intentions are toward maintaining this library. I appreciate it's been 10 years since the last commit and your circumstances may have changed. Open source maintainers do not owe anyone anything but I do think you have a responsibility to either fix this or if you do not have the time, at least remove it from packagist and make the repo private so the bug cannot continue to affect projects.

My offer of help is on the table if you need it.