Testing Luhn algorithm with JavaScript

[VillaRava]

I have created some JavaScript-code that tests a number according to the Luhn algorithm. It works the way it should. However, it is rather lengthy. I’m wondering if there’s a smart way to shorten the code? Sure, there are other ways to write the whole thing, but I do like this particular for-loop and the charAt() method, they are comprehensible to me. I would prefer to maintain the “tools” but still cut back on functions and possibly variables.

function luhnTest(userInput){

	var backwards = "";
	var multiplyx2 = "";
	var total = 0;
	
	/* For-loop going through the number. It begins at the second digit from the end.
	Then it adds to "backwards" every other digit going backwards through the number. */ 
	for(i = userInput.length-2; i >= 0; i-=2){
		backwards += userInput.charAt(i);
	}	
	
	/* For-loop going through the digits in "backwards" and multiplying them by 2. 
	The multiplied digits are stored in "multiplyx2".*/
	for(i = 0; i < backwards.length; i++){
		multiplyx2 += backwards.charAt(i)*2;
	}	
	
	/* The digits in "multiplyx2" are added up and stored in "total". */ 
	for(i = 0; i < multiplyx2.length; i++){
		total += parseInt(multiplyx2.charAt(i));
	}
	
	/* Adding up the digits that have been left out. Starting with the third digit from the end.
	These digits are added to "total". */
	for(i = userInput.length-3; i >= 0; i-=2){
		total += parseInt(userInput.charAt(i));
	}
	
	/* Adding the last digit in the card number to "total".
	Could have done so in previous loop but just for the sake of it (that's how the Luhn algorithm goes) . */
	total += parseInt(userInput.charAt(userInput.length-1))
	
	/* Testing if "total" can be divided by 10 without leaving a remainder. */		
	if((total % 10) == 0){
		return true;
	}	
	
}

[felgall]

I’d start by splitting the number so the separate digits are separate entries in an array and then use array methods to process it instead of loops.

I’d also replace parseInt with + since you are not trying to change number bases.

[markbrown4]

Without the comments it’s not all too lengthy.

Every function should return a value, because your return is in a condition when that isn’t true it will return undefined

if ((total % 10) == 0) {
    return true;
}

can be written as this so it will return true or false

return (total % 10) == 0;

–

You say that you like the semantics of the for loop, that’s fine but there’s other ways to achieve the same thing.

for (i = 0; i < backwards.length; i++) {
  multiplyx2 += backwards.charAt(i)*2;
}

This operation is called mapping, you’re running an operation on every item in an array(or string), that same operation can be more elegantly expressed using a functional approach.

numbers.map((i)=> i * 2)

Similarly

for (i = 0; i < multiplyx2.length; i++){
  total += parseInt(multiplyx2.charAt(i));
}

Can be expressed better using reduce, which turns an array of values into a single value

numbers.reduce((a, b)=> a + b)

Those functional tricks are worth learning about.

–

Also, always var your i variables. When you miss a var it creates global variables.

[felgall]

@markbrown4

When you miss a var it creates global variables.

That’s a good reason for specifying ‘uise strict’; as that generates a syntax error for a missing var instead of a global variable.

[Paul_Wilkins]

The luhn number can be coded as tightly as follows:

var luhn10 = function(a,b,c,d,e) {
  for(d = +a[b = a.length-1], e=0; b--;)
    c = +a[b], d += ++e % 2 ? 2 * c % 10 + (c > 4) : c;
  return !(d%10)
};

If you want something more readable though, rosetta code has several luhn number code examples in a huge variety of languages, including JavaScript.

I prefer this version:

function luhn(str) {
    return str.split('').reduceRight(function (prev, curr, idx) {
        prev = parseInt(prev, 10);
        if ((idx + 1) % 2 !== 0) {
            curr = (curr * 2).toString().split('').reduce(function (p, c) {
                return parseInt(p, 10) + parseInt(c, 10);
            });
        }
        return prev + parseInt(curr, 10);
    }) % 10 === 0;
}

[felgall]

@Paul_Wilkins

I prefer this version:

function luhn(str) {
return str.split(‘’).reduceRight(function (prev, curr, idx) {
prev = parseInt(prev, 10);
if ((idx + 1) % 2 !== 0) {
curr = (curr * 2).toString().split(‘’).reduce(function (p, c) {
return parseInt(p, 10) + parseInt(c, 10);
});
}
return prev + parseInt(curr, 10);
}) % 10 === 0;
}

I’d shorten that slightly to:

function luhn(str) {
    return str.split('').reduceRight(function (prev, curr, idx) {
        if ((idx + 1) % 2 !== 0) {
            curr = (curr * 2).toString().split('').reduce(function (p, c) {
                return (+p) + (+c);
            });
        }
        return (+prev) + (+curr);
    }) % 10 === 0;
}

or at least use Number() for the string to number conversions (as that’s the right function for doing that, not parseInt).

[Paul_Wilkins]

@felgall

or at least use Number() for the string to number conversions (as that’s the right function for doing that, not parseInt).

Yes, it comes down to whether you want type conversion with Number(), or parsing with parseInt()

I agree that Number() is better in this case, but I disagree with using the + prefix. That’s too close to being a side-effect, and makes the code more difficult for other coders to understand the intention.

return Number(prev) + Number(curr);

The saving of a few keystrokes isn’t as important as clarity to others.

[VillaRava]

Tried “+= +” and that works as well. Such as:

total += + userInput.charAt(i);

But I can’t read it. What does “+= +” mean?

[markbrown4]

I’m with Paul on that, don’t use + to turn a string into a number. It’s a trick and there’s a reason you don’t know what it does, it’s not obvious enough.

[Paul_Wilkins]

@VillaRava

total += + userInput.charAt(i);

Just use Number() to convert the character.

total += Number(userInput.charAt(i));

It’s a lot easier for everyone to understand that.

[VillaRava]

Right! Two questions:

1. How is Numbers better than parseInt?

2. I declared “backwards” and “multiplyx2” as empty arrays. I still have to transform the chars to numbers. Such as:

total += Numbers(multiplyx2.charAt(i));

Why is that? Aren’t they already numbers?

[Paul_Wilkins]

[quote=“VillaRava, post:11, topic:221093, full:true”]

1. How is Numbers better than parseInt?[/quote]

With Numbers and parseInt, you have the following ways that they are used:

var num1 = Number("4"),
    num2 = parseInt("4", 10);

The first one with Number uses type conversion, so Number(“4x”) results in NaN
The second one with parseInt uses parsing, so parseInt(“4x”, 10) results in 4. parseInt pulls out the first number that it finds in the string.

Because parseInt is a bit more complex to use, and we’re not wanting to convert a fractional number to an integer, it’s better to remain with the simpler type conversion technique with Number(“4”).

@VillaRava

2. I declared “backwards” and “multiplyx2” as empty arrays. I still have to transform the chars to numbers. Such as:

total += Numbers(multiplyx2.charAt(i));

Why is that? Aren’t they already numbers?

Where you have used Numbers, that should instead be Number.

You have filled the multiplex2 array with characters. When you add “3” + “4” you end up with “34” and not 7.

It’s best to make it clear that this place is where the character is now being treated as a number. As such, using Number(multiplyx2.charAt(i)); is a better and clean option.

[m3g4p0p]

@markbrown4

I’m with Paul on that, don’t use + to turn a string into a number. It’s a trick and there’s a reason you don’t know what it does, it’s not obvious enough.

I’d disagree. You should always use the double bitwise NOT just to let everybody know what clever kind of guy you are. :~~)

[Paul_Wilkins]

@m3g4p0p

I’d disagree. You should always use the double bitwise NOT just to let everybody know what clever kind of guy you are. :~~)

Them’s fighting words there

I’ll replace your double bitwise NOT with fully fledged functions that cater for every tiny side-effect that double bitwise NOT’s are capable of handling, then find that a business-wide code quality directive has been ordered.

Some other programmer will look at my highly complex function and ask questions, before finally realising that it’s all just to handle the conversion of a single character to a number and replace my finely crafted function with a simple number(str).

I will create a song and dance about the loss of my code, the finger of blame will be pointed squarely at me, and thousands of business dollars will have been lost in the process.

The accounts department then become involved wanting to recoup the loss, backed up by a fleet of lawyers with extremely thin briefcases, and when questions are asked about how this could happen I will tell them about a clever kind of guy that I used to know . . .