4
\$\begingroup\$

I need to implement a method that adds a "7-3-1" check digit to my id. The "7-3-1" method works as following:

  1. Weights 7, 3, 1 are assigned to the original number from right to left. So if the original number is 12345, then 5 will have weight 7, 4 will have weight 3, 3 will have weight 1, 2 will have weight 7, 1 will have weight 3.

  2. Numbers are multiplied by their weight and added to each other. So 5*7 + 4*3 + 3*1 + 2*7 + 1*3 = 67.

  3. Sum is rounded to next full tenth and then from that sum is subsctracted to get the weight. So 70 - 67 = 3.

And the number with check digit would be 123453.

Here is what I have at the moment:

class ReferenceNumberUtil
{
    private static $offset = 0; // possiblity to set offset

    public static function referenceNumberFromId($id)
    {
        $id = $id + ReferenceNumberUtil::$offset;
        $weights = [7, 3, 1];
        $sum = 0;
        foreach (array_reverse(str_split($id)) AS $index => $digit) {
            $weightIndex = $index % 3;
            $sum += $digit * $weights[$weightIndex];
        }
        $checkDigit = (ceil($sum / 10) * 10) - $sum;
        return $id . $checkDigit;
    }

    public static function idFromReferenceNumber($number)
    {
        $numberWithoutCheckDigit = substr($number, 0, -1);
        return $numberWithoutCheckDigit - ReferenceNumberUtil::$offset;
    }
}

I've ran some tests and as far as i can tell this works correctly. But i'm still kind of hesitant. I would appreciate a review of this code to be certain that it works correct in all situations and also any ideas on how to improve it further are also welcome.

\$\endgroup\$
1
\$\begingroup\$
  • What is the purpose of $offset? It doesn't seem to do anything here.

  • What is the possible range of values of $id? Do you need to check that it is an integer, and that it is not negative?

  • Can $id have leading zeroes? That would naturally mess up the alignment with the 7 3 1 array, and possibly interfere with using str_split.

  • I would personally be very nervous about mixing around types like this, treating things as integers, then strings, then integers, then strings again.... At a minimum I would recommend being explicit about every time you do so. Otherwise it's far from clear what is going on.

Sum is rounded to next full tenth and then from that sum is subsctracted

I would personally always prefer to use mod for this sort of thing, partly because messing with floating point numbers is confusing and partly just because the operation is inherently a mod calculation. For example something like this works as long as $sum is at least 1:

$checkDigit = 9 - (($sum - 1) % 10)
\$\endgroup\$
  • \$\begingroup\$ Thanks for you anwser. The $id that is passed in is actually a autoincremented database row number. Offset is something i would actually set to 1000 or more first users wont get a too small of a number. Thought its good to leave in in case there is a mistake there, but i set it to 0 so it would be clearer. Id is unsigned int(10) so negatives and leading zeroes are not a problem. Thanks for the mod example, ill use it. Also im not really sure what you mean about data types, could you please show an example how you would do it with datatypes? \$\endgroup\$ – MarkoJ May 9 '18 at 15:32
  • \$\begingroup\$ I just mean that when you have, say, an integer and you're treating it as a string, explicitely using strval would make it clearer that that is what you're doing. \$\endgroup\$ – Josiah May 10 '18 at 6:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.