Validating a credit card number using Luhn's algorithm

Question

I implemented Luhn's algorithm explained on Wikipedia to validate a credit card number, in Bash.

Does this code follow good practices?

#!/bin/bash
getchecknumber(){ # get the last digit of the cc number
    str=$1
    checknum=$(echo ${str: -1})
}
getrest(){ # get the rest of the numbers in the sequence
    str=$(echo ${str%?})
}
doubleeveryother(){ # double every other number
    # split characters into an array
    arr=()
    i=0
    while [ "$i" -lt "${#str}" ]; do
        arr+=(${str:$i:1})
        i=$((i+1))
    done
    for ((i=1; i<=15; i++))
    do  {
        char=$(echo "$i-1" | bc)
        double=$(echo "${arr[$char]} * 2" | bc)
        if [ $(($i%2)) -eq 0 ] ; then foo=bar; else arr[$char]=$double #replace the number in the array
        fi
    }
    done
    echo -e
}
adddigitsum() { #split a two digit number into two numbers and add them
    for ((a=0; a<=14; a++))
    do {

        NUMSPLIT=()
        number=$(echo ${arr[$a]})
        if [[ ${#number} == 2 ]] ; then
            i=0
            while [[ "$i" -lt ${#number} ]]; do
                NUMSPLIT+=(${number:$i:1})
                i=$((i+1))
                done
            num=$((${NUMSPLIT[0]} + ${NUMSPLIT[1]}))
            addeddigit=$( echo $num | bc )
            arr[$a]=$addeddigit # replace the number in the array
        fi
    }
    done
}
addall(){ #add all the digits
    sum=$( IFS="+"; bc <<< "${arr[*]}" )
}
multiply(){ #multiply by 9
    result=$( echo "$sum * 9" | bc )
}
validate(){ #see if the last number of 'result' is equal to the checknum
    resultchecknum=$(echo ${result: -1})
    if [[ $resultchecknum == $checknum ]] ; then echo "CC Number is valid!" ; else echo "CC Number is not valid."
    fi
}
echo "$1 "
getchecknumber $1
getrest
doubleeveryother
adddigitsum
addall
multiply
validate

Answer 1

if [ $(($i%2)) -eq 0 ] ; then foo=bar; else arr[$char]=$double

Then foo=bar? In any case, an alternative way of writing this is something like (borrowing your final if statement):

[[ $resultchecknum == $checknum ]] && echo "CC Number is valid!" \
                                    || echo "CC Number is not valid."

Also, you have a mix of echo "..." | bc and bc <<< "...", I suggest keeping to the latter as it simpler to write.

A similar alternative to if-then-else is using &&:

[ ! $(($i%2)) -eq 0 ] && arr[$char]=$double

Answer 2

Does this code follow good practices?

It has some violations.

Avoid sub-shells when you don't need one

Instead of this:

str=$1
checknum=$(echo ${str: -1})

You can use parameter expansion (${...}) directly without a sub-shell:

str=$1
checknum=${str: -1}

The same goes for the similar use in the getrest function.

Arithmetic operations

Instead of this:

while [ "$i" -lt "${#str}" ]; do
    arr+=(${str:$i:1})
    i=$((i+1))
done

It's better to use ((...)) for arithmetic operations, like this:

while (( i < ${#str} )); do
    arr+=(${str:$i:1})
    ((++i))
done

Empty then in if statements

I see you added a dummy assignment foo=bar in this if statement because the then clause cannot be empty:

if [ $(($i%2)) -eq 0 ] ; then foo=bar; else arr[$char]=$double; fi

In such situation, it's better to negate the condition:

if ! [ $(($i%2)) -eq 0 ] ; then arr[$char]=$double; fi

Odd groupings

The grouping with {...} is strange and unnecessary here:

while condition
do {
    # ...
}
done

Unless you need the {...} for something, then it's better to write simpler:

while condition
do
    # ...
done

Avoid echo -e

The various flags of echo are not portable. Avoid them when possible.

The command echo -e without other arguments is exactly the same as echo. So just write echo and that's it.

Answer 3

In addition to Janos' points (which are all very good points) it should be noted that variables in Bash are untyped (in other words - a string that is made up of numerics can be used in arithmetic). So the conversion from string to array is unnecessary.

With that in mind (and because I found this question whilst looking for a Bash implementation of the Luhn check) I thought I'd provide what I ended up using:

#!/bin/bash

pan=$1
panlen=${#pan}

for i in $(seq $((panlen - 1)) -1 0); do
  digit=${pan:$i:1}
  if [ $(((panlen-i) % 2)) -eq 0 ]; then
     #even
     ((digit*=2))
     [ ${#digit} -eq 2 ] && digit=$((${digit:0:1}+${digit:1:1}))
  fi
  ((sum+=digit))
done

[ $((sum % 10)) -eq 0 ] || exit 1

This script takes a single parameter (the PAN) and exits with 1 if the PAN is invalid. For example, if we place this script in a file called isLUHNValid.sh we can run it with a valid PAN like:

./isLUHNValid.sh 4388576018410707 && echo 'Valid PAN' || echo 'Invalid PAN'

which will echo 'Valid PAN'.

And with an invalid PAN:

./isLUHNValid.sh 4388576018410708 && echo 'Valid PAN' || echo 'Invalid PAN'

it will echo 'Invalid PAN'.