HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMajor

Check the validity of a South African ID number using the Luhn Algorithm

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
africansouththenumberalgorithmvalidityusingcheckluhn

Problem

import java.math.BigInteger;

/**
 * Created by lungisani on 2017/02/25.
 */
public class Luhn {
    public Boolean getIdentitySummation(BigInteger identities){
        String identify = String.valueOf(identities);
        int sumOdd = 0, sumEven = 0, _doubled;
        int summation;
        try {
            String identity = String.valueOf(identify);
            int length = identity.length() - 1;
            char[] chars = identity.toCharArray();
            String check = identity.substring(12);
            for(int x  = 0; x  9){
                            _doubled = doubled - 9;
                        }else{
                            _doubled = doubled;
                        }
                        sumEven += _doubled;
                }
            }
            summation = sumOdd + sumEven;
            int checksum = Integer.valueOf(check);
            if((summation * 9) % 10 == checksum)
                return Boolean.TRUE;
            else
                return Boolean.FALSE;
        } catch (Exception e) {
            e.printStackTrace();
            return Boolean.FALSE;
        }
    }
}

Solution

Let's work through some of the simpler issues, like code redundancy, and some helper-functions in the core library that help a lot, and will make your code simpler.

Double-checking else

When you have an either-or check in an if-condition, there's no need to double-check the else side of things.

you check for odd-positioned digits, which implies every other digit is even-positioned... This code:

if(x % 2 == 0 ){
    .....
}else if(x % 2 != 0){
    .....
}


can be just:

if(x % 2 == 0 ){
    .....
} else {
    .....
}


Try-Catch

There's no code in your function that throws an explicit exception - why do you have a try/catch? All integer parsing comes from digits in a BigInteger, so there can be no illegal characters, etc.

Character-to-digit-value

Your code has a lot of this type of logic:

String numString = String.valueOf(chars[x]);
int numbers = Integer.valueOf(numString);


but that can be simplified to just:

int numbers = Character.getNumericValue(chars[x]);


Separated sums

You have both sumOdd and sumEven, but there's no need for both. You can have one sum accumulator and use it in each side.

Simplifying the doubled digits

The algorithm requires doubles that are larger than 9 to be reduced by 9. Your code is:

int doubled = numbers * 2;
if(doubled > 9){
    _doubled = doubled - 9;
}else{
    _doubled = doubled;
}
sumEven += _doubled;


Using a "ternary" expression, and a little bit of manipulation on the math, you can reduce that to just:

sum += digit < 5 ? digit * 2 : digit * 2 - 9;


Autoboxing and conditionals

Java will "autobox" primitive variables like boolean to their full class types Boolean when needed, without any explicit handling. Let's take this exit segment:

if((summation * 9) % 10 == checksum)
    return Boolean.TRUE;
else
    return Boolean.FALSE;


This should have braces on the 1-liners, to be:

if((summation * 9) % 10 == checksum) {
    return Boolean.TRUE;
} else {
    return Boolean.FALSE;
}


but really, that's all unnecessary, because autoboxing comes to the rescue:

return (summation * 9) % 10 == checksum


That's all you need.

Conclusion

Putting all these suggestions together, you can significantly reduce the complexity of the code, to something like:

public static Boolean getIdentitySummationRL(BigInteger identities){
    char[] idchars = identities.toString().toCharArray();
    int sum = 0;
    // loop over each digit, except the check-digit
    for (int i = 0; i < idchars.length - 1; i++) {
        int digit = Character.getNumericValue(idchars[i]);
        if ((i % 2) == 0) {
            sum += digit;
        } else {
            sum += digit < 5 ? digit * 2 : digit * 2 - 9;
        }
    }
    int checkdigit = Character.getNumericValue(idchars[idchars.length - 1]);
    int compdigit = (sum * 9) % 10;

    return checkdigit == compdigit;
}


(I have checked that using my ID number, and it's OK)

Update

Note I have been doing a little more reading on Luhn's algorithm because of @Molvalio's comment and also I remember doing it a couple of decades ago for other numbers (not ID numbers) and I remember the checking algorithm to be different to this implementation. I was right that checking the number is simpler than your code. See the algorithm here: Luhn's Algorithm Verification

The point is that your code is computing a check-digit and comparing it with the existing digit, but you're missing the fact that the digit is designed to be incorporated in to the same calculations as the checksum, and a valid number has a resulting digit of 0.

Additionally, the Luhn's algorithm is computed right-to-left. In your case, because the length of SA ID numbers is 13, you're OK (both left and right digits are odd) but you should implement the algorithm more closely....

So, the code can be simplified further:

public static Boolean checkLuhn(BigInteger identities){
    char[] idchars = identities.toString().toCharArray();
    int sum = 0;
    // loop over each digit right-to-left, including the check-digit
    for (int i = 1; i <= idchars.length; i++) {
        int digit = Character.getNumericValue(idchars[idchars.length - i]);
        if ((i % 2) != 0) {
            sum += digit;
        } else {
            sum += digit < 5 ? digit * 2 : digit * 2 - 9;
        }
    }
    return (sum % 10) == 0;
}

Code Snippets

if(x % 2 == 0 ){
    .....
}else if(x % 2 != 0){
    .....
}
if(x % 2 == 0 ){
    .....
} else {
    .....
}
String numString = String.valueOf(chars[x]);
int numbers = Integer.valueOf(numString);
int numbers = Character.getNumericValue(chars[x]);
int doubled = numbers * 2;
if(doubled > 9){
    _doubled = doubled - 9;
}else{
    _doubled = doubled;
}
sumEven += _doubled;

Context

StackExchange Code Review Q#157069, answer score: 20

Revisions (0)

No revisions yet.