# Validating a credit card number using Luhn's algorithm

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  ## 3 Answers 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  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. 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'.

• Good point about untyped variables, and welcome to Code Review. Btw if you post your version up for review, I'll have a few comments on it ;-) May 27, 2016 at 7:45
• Thanks. I'd actually love feedback. Should I make a new post? Being new here I'm not really sure of the shoulds and should nots. If I make a new post - do I just say "Here's a script I wrote. Looking for feedback"? May 27, 2016 at 11:54
• Something like that, but do read the how-to-ask and on-topic pages first May 27, 2016 at 12:03
• Thanks again Janos. I've posted my version here: codereview.stackexchange.com/questions/129649/… May 30, 2016 at 4:35