2
\$\begingroup\$

So I am using the following

<?php
function LuhnCalc($number) {
  $chars = array_reverse(str_split($number, 1));
  $odd = array_intersect_key($chars, array_fill_keys(range(1, count($chars), 2), null));
  $even = array_intersect_key($chars, array_fill_keys(range(0, count($chars), 2), null));
  $even = array_map(function($n) { return ($n >= 5)?2 * $n - 9:2 * $n; }, $even);
  $total = array_sum($odd) + array_sum($even);
  return ((floor($total / 10) + 1) * 10 - $total) % 10;
}
print LuhnCalc($_GET['num']);
?>

however it seems that BPAY which is version 5 of MOD 10 which for the record I cant find what the documentation for. seems to not be the same as MOD10

the following numbers where tested 2005,1597,3651,0584,9675

bPAY
2005 = 20052
1597 = 15976
3651 = 36514
0584 = 05840
9675 = 96752

MY CODE

2005 = 20057 
1597 = 15974 
3651 = 36517 
0584 = 05843 
9675 = 96752

as you can see none, of them match the BPAY numbers

\$\endgroup\$
1
  • 2
    \$\begingroup\$ This may get better traction on StackOverflow where there's a much larger user base. (And this is off topic for CR as CR questions are expected to contain code that currently works.) \$\endgroup\$ – Corbin Jun 17 '12 at 7:01
2
\$\begingroup\$

First off, welcome to CodeReview, where your questions may take a while to get answered. Hang in there, we'll get to them. I had to look this up because I've never heard of it before, but my math isn't half bad, so I'm confident in my answer.

Your check numbers are mostly fine. I'd be more concerned about those your checking against. By mostly, I mean that the return value for the last is zero not two. That last number is probably just a typo on your part because you clarify that none of your numbers matches those that you're comparing them to, yet that last one does. Also, for completeness I'd like to mention that if you do not pass these numbers as strings to your function you could get undesired results. I found that out the hard way with your third number. I was getting range out of bounds errors because str_split() only returned an array with one value. This is because, for some reason, the leading zero deleted all but the five when passed to the function. Odd that, I'll have to look further into it. Anyways, make sure the numbers you pass to your function are strings to avoid this error.

Now, I do have some suggestions to make.

The luhn algorithm states that a zero prepended to the beginning of an odd lengthed string will allow you to process it without corrupting the data. Not that this matters in this instance, you are only processing four digit numbers. Either way, that array reversal is unnecessary. Should you start needing to process multi-length integers, then you can just prepend the zero.

This is so much more convoluted than it needs to be... You don't need two separate arrays, you can do this with one and a loop. Which also removes the need for array_map.

$length = count( $chars );
for( $i = 0; $i < $length; $i++ ) {
    if( $i % 2 ) {
        $value = $chars[ $i ] * 2;
        $chars[ $i ] = $value >= 10 ? $value - 9 : $value;
    }
}

If you are trying to avoid loops and if statements, don't. I find the above much easier to read than your run-on PHP functions. Even if it does save processing time, not sure if it does, I don't think it'd be worth it.

Your formula is also more convulted than it needs to be. I've not seen the one you're currently using before. Not that that's saying much, I did just say I've only recently looked it up. But there are two simpler ones. The first, is easier for us to do in our head than program so I wont describe it here, look it up on wikipedia. But the second is easy enough to program and goes like this: perform the above loop. I'm assuming you know what it does so I won't explain it here. Sum up all array values. Multiply that by 9. And your answer is the modulo of 10. So $total * 9 % 10. Much easier than ( floor( $total / 10 ) + 1 ) * 10 - $total ) % 10. You can even do that one in your head if pressed.

Hope this helps!

EDIT:

Here's my suggestions in code.

function LuhnCalc( $number ) {
    $chars = str_split( $number );
    $length = count( $chars );
    if( $length % 2 ) {
        $chars = '0' . $chars;//should work, not tested
        //$chars = array_reverse( $chars );
    }
    for( $i = 0; $i < $length; $i++ ) {
        if( $i % 2 ) {
            $value = $chars[ $i ] * 2;
            $chars[ $i ] = $value >= 10 ? $value - 9 : $value;
        }
    }
    $total = array_sum( $chars );
    return $total * 9 % 10;
}
\$\endgroup\$
2
  • \$\begingroup\$ Sorry I tried what you said, and I could not get it working, Could you do a sample script please. \$\endgroup\$ – RussellHarrower Jun 17 '12 at 6:10
  • \$\begingroup\$ I've created it using the suggestions I mentioned and added it to the edit section on the bottom of my answer. \$\endgroup\$ – mseancole Jun 18 '12 at 13:10

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