Refactored 2 functions into 1 - Verify UPC/EAN13 barcode I refactored 2 seperate functions that I found here into a single function:

&lt;?
    function isValidBarcode($barcode) {

        $barcode = trim($barcode);

        //If value is not numeric, it is not valid.
        if ((!is_numeric($barcode)) &amp;&amp; ((strlen($barcode) == 12) || (strlen($barcode) == 13))) return false;

        //set empty variables
        $even_sum = $odd_sum = 0;

        //array key value to count to, doesn't include the last 2 numbers of barcode
        $countTo = strlen($barcode) - 2;

        //loop through every digit in barcode, count even and odds based on $countTo set before.
        for ($x = 0; $x &lt;= $countTo; $x++) {
            if ($x % 2 == 0) {
                $even_sum += $barcode[$x];
            } else {
                $odd_sum += $barcode[$x];
            }
        }

        //Multiply even_sum by 3, and then add odd_sum to it.
        $total_sum   = $even_sum * 3 + $odd_sum;

        //get highest multiple of 10 of total_sum.
        $next_ten    = ceil($total_sum / 10) * 10;

        //Subtract highest multiple of 10 from total_sum to get check_digit.
        $check_digit = $next_ten - $total_sum;

        //if check_digit is equal to the last digit in the barcode, it is valid.
        if ($check_digit == $barcode[$countTo + 1]) return true;

        //if check_digit is not equal to the last digit in the barcode, it is invalid.
        return false;
    }

?&gt;


But, I feel as though I can make this even better. I wanted some other opinions on this, and I wanted to see what other people could come up with in terms of a solution.

My main question is, how efficient is this code, and is it possible for me to make it more efficient?
Some thoughts:


Why would you silently trim() and potentially transform an invalid bar code value into a valid one?  This would seem to give unexpected results.  If a string value needs to be trimmed, let the caller do it before calling this method.
What happens if you are not passed a string? To me this might be an exceptional event and worthy of throwing exception from this function, as this is something the developer writing code calling this function needs to be aware of.
Do you really want to use is_numeric() to validate the input format?  This can lead to false positives (for example +123.45678e9 would pass your current format check). If what you are expecting are 12 or 13 digits in this string, then precisely test for that. In addition to your questionable use of is_numeric(), you also have your length conditions set inappropriately.  You should be using OR between the digit criteria and length criteria as either cause failure.  Right now a string like foobar will actually pass your conditional because it does not satisfy both conditions.


For example:

if (!preg_match('/^[0-9]{12,13}$/', $barcode)) {
    return false;
}
// rest of code


or

if (!ctype_digit($barcode) || $bcLen &lt; 12 || $bcLen &gt; 13) {
    return false;
}
// rest of code



I think the logic to determine check digit can be done in a more simplified manner. Part of this simplificatoin could be to explictly convert the barcode string to an array to make it easier to work with. I have done a function rewrite below which I hope demonstrates some simplification.


Example:

function isValidBarcode($barcode)
{
    if (!is_string) {
        throw new InvalidArgumentException('String value expected.');
    }
    if (!preg_match('/^[0-9]{12,13}$/', $barcode)) {
        return false;
    }
    $barcodeArray = str_split($barcode);
    $lastDigit = (int)array_pop($barcodeArray);
    $sum = 0;
    for($i = 0; $i &lt; count($barcodeArray); $i++) {
        $sum += (int)$barcode[$i];
        if ($i % 2 === 0) {
            $sum += (int)$barcode[$i] * 2;
        }
    }
    $checkDigit = 10 - ($sum % 10);
    return $lastDigit === $checkDigit;
}



Be careful using loose comparisons as your default means for comparing.  I would suggest you get in the habit of using exact comparisons as default unless there is good reason to do loose comparison.  I don't see any actual bug in this code (since PHP's built-in type juggling should take care of string to integer conversions in your specific use case here).  Bu,t you might not always be so lucky. PHP's loose typing can be useful, but it can also be a source of hard-to-identify bugs.  To me when reading code, I would like the code writer to explicitly demonstrate their understanding of types being applied to the variables.
Style-wise, I have a few concerns.  You are mixing camelCase and snake_case in your code.  Ideally you should pick one style and stick to it (though PHP as a language is not very good about this).  There are too many comments, most of which are not really necessary. You also have inconsistent spacing around your = assignments near the end of code. Trying to horizontally-align assignment operators can be a bit of a fool's errand if you try to do this throughout your codebase. Every time you want to make changes to variable names and such, you end up having to redo spacing.  Is it worth it?
Why are you concerned with the "efficiency" of this code?  Are you planning to execute this code thousands of time in any one script execution such that every millisecond spent in this function gets compounded?  If not, you might be prematurely optimizing this bit of code.  Sure, you want to make sure it runs with reasonable efficiency (which I think your code does since it is such a simple data structure - a string - that you are working with), but I don't think you want to necessarily squeeze out every millisecond out of this function if it means making the function hard to read or maintain.  That code quality aspect should likely be you primary concern with a function such as this which you are not likely to spend a large percentage of your overall processing time in.