UPC-A Validation

Question

I've just written a validation routine for a UPC-A, which ensures the check digit matches the given UPC number (according to rules I found on Wiki, mind!).

Below is the code, what do you think?

function valid_upc_a($value)
{
    $odd_sum = $even_sum = 0;
    if(strlen($value) != 12) return FALSE;
    $chars = str_split($value);
    for($i=0;$i<11;$i++)
    {
        $odd_sum += $i%2==0?$chars[$i]:0;
        $even_sum += $i%2==1?$chars[$i]:0;
    }
    $total_sum = $even_sum + $odd_sum*3;
    $modulo10 = $total_sum % 10;
    $check_digit = 10 - $modulo10;
    return (int)$chars[11] === $check_digit;
}

It works for a couple of cases I made up, here it is on CodePad:

http://codepad.viper-7.com/QEqpFX

And the checkdigit derivation algorithm is described here:

http://en.wikipedia.org/wiki/Universal_Product_Code#Check_digits

Answer 1

I have made a cleaner version of yours put something in and get rid of other things:

<?php

function valid_upc_a($value) {
    $upc = strval($value);

    if(!isset($upc[11])) {
        return FALSE;
    }

    $odd_sum = $even_sum = 0;

    for($i = 0; $i < 11; ++$i) {
        if ($i % 2) {
            $even_sum += $upc[$i];
        } else {
            $odd_sum += $upc[$i];
        }
    }

    $total_sum = $even_sum + $odd_sum * 3;
    $modulo10 = $total_sum % 10;
    $check_digit = 10 - $modulo10;

    return $upc[11] == $check_digit;
}

It's not a huge modification but for example to me is more readable.

Answer 2

Two things I noticed.

1. The return statement if the length is invalid should be on a separate line, and wrapped in braces
2. What purpose does the int cast solve? I thought php was typeless.

Other than that, the only other thing I noticed is to space out your operators. However, all the actual function seems to work fine