Can't figure out the error Luhn check

Question

Its supose to tell me if a card is valid or invalid using luhn check 4388576018402626 invalid 4388576018410707 valid but it keeps telling me that everything is invalid :/ Any tips on what to do, or where to look, would be amazing. I have been stuck for a few hours. It would also help if people tell me any tips on how to find why a code is not working as intended. im using eclipse and java

public class Task11 {

   public static void main(String[] args) {
        Scanner input = new Scanner(System.in);

        System.out.print("Enter a credit card number as a long integer: ");
        long number = input.nextLong();


        if (isValid(number)) {
            System.out.println(number + " is valid");
        } else {
            System.out.println(number + " is invalid");
        }


    }

    public static boolean isValid(long number) {

        return (getSize(number) >= 13) && (getSize(number) <= 16)
                && (prefixMatched(number, 4) || prefixMatched(number, 5) || prefixMatched(number, 6) || prefixMatched(number, 37))
                && (sumOfDoubleEvenPlace(number) + sumOfOddPlace(number)) % 10 == 0;
    }

    public static int sumOfDoubleEvenPlace(long number) {

        int result = 0;

        long start = 0;

        String digits = Long.toString(number);

        if ((digits.length() % 2) == 0) {
            start = digits.length() - 1;
        } else {
            start = digits.length() - 2;
        }


        while (start != 0) {


            result += (int) ((((start % 10) * 2) % 10) + (((start % 10) * 2) / 2));

            start = start / 100;

        }

        return result;
    }


    public static int getDigit(int number) {

        return number % 10 + (number / 10);

    }

    public static int sumOfOddPlace(long number) {

        int result = 0;

        while (number != 0) {

            result += (int) (number % 10);

            number = number / 100;

        }

        return result;

    }

    public static boolean prefixMatched(long number, int d) {

        return getPrefix(number, getSize(d)) == d;

    }

    public static int getSize(long d) {

        int numberOfDigits = 0;

        String sizeString = Long.toString(d);
        numberOfDigits = sizeString.length();

        return numberOfDigits;

    }


    public static long getPrefix(long number, int k) {

        String size = Long.toString(number);

        if (size.length() <= k) {
            return number;
        } else {
            return Long.parseLong(size.substring(0, k));
        }
    }
}

Answer 1

You should modiffy your isValid() method to write down when it doesn't work, like this:

 public static boolean isValid(long number) {
        System.err.println();
        if(getSize(number) < 13){
            System.out.println("Err: Number "+number+" is too short");
            return false;
        } else if (getSize(number) > 16){
  public static boolean isValid(long number) {
        System.err.println();
        if(getSize(number) < 13){
            System.out.println("Err: Number "+number+" is too short");
            return false;
        } else if (getSize(number) > 16){
            System.out.println("Err: Number "+number+" is too long");
            return false;
        } else if (! (prefixMatched(number, 4) || prefixMatched(number, 5) || prefixMatched(number, 6) || prefixMatched(number, 37)) ){
            System.out.println("Err: Number "+number+" prefix doesn't match");
            return false;
        } else if( (sumOfDoubleEvenPlace(number) + sumOfOddPlace(number)) % 10 != 0){
            System.out.println("Err: Number "+number+" doesn't have sum of odd and evens % 10. ");
            return false;
        }  
        return true;
    }

My guess for your problem is on the getPrefix() method, you should add some logs here too.

EDIT: so, got more time to help you (don't know if it's still necessary but anyway). Also, I corrected the method I wrote, there were some errors (like, the opposite of getSize(number) >= 13 is getSize(number) < 13)... First it will be faster to test with a set of data instead of entering the values each time yourself (add the values you want to check):

public static void main(String[] args) {
           long[] luhnCheckSet = {
                   0, // too short
                   1111111111111111111L, // too long (19)
                   222222222222222l // prefix doesn't match
                   4388576018402626l, // should work ?
                   };

        //System.out.print("Enter a credit card number as a long integer: ");
        //long number = input.nextLong();

        for(long number : luhnCheckSet){
            System.out.println("Checking number: "+number);
            if (isValid(number)) {
                System.out.println(number + " is valid");
            } else {
                System.out.println(number + " is invalid");
            }
            System.out.println("-");
        }
    }

I don't know the details of this, but I think you should work with String all along, and parse to long only if needed (if number is more than 19 characters, it might not parse it long). Still, going with longs.

I detailed your getPrefix() with more logs AND put the d in parameter in long (it's good habit to be carefull what primitive types you compare):

public static boolean prefixMatched(long number, long d) {
            int prefixSize = getSize(d);
            long numberPrefix = getPrefix(number, prefixSize);
            System.out.println("Testing prefix of size "+prefixSize+" from number: "+number+". Prefix is: "+numberPrefix+", should be:"+d+", are they equals ? "+(numberPrefix == d));
            return numberPrefix == d;
        }

Still don't know what's wrong with this code, but it looks like it comes from the last test: I didn't do it but you should make one method from sumOfDoubleEvenPlace(number) + sumOfOddPlace(number)) % 10 and log both numbers and the sum (like i did in prefixMatched() ). Add logs in both method to be sure it gets the result you want/ works like it should. Have you used a debugger ? if you can, do it, it can be faster than adding a lot of logs !

Good luck

Answer 2

EDIT:

Here are the working functions and below I provided a shorter, more efficient solution too:

public class CreditCardValidation {

  public static void main(String[] args) {
 Scanner in = new Scanner(System.in);
    int count = 0;
    long array[] = new long [16];
   do
   {
    count = 0;
    array = new long [16];
    System.out.print("Enter your Credit Card Number : ");
    long number = in.nextLong();
    for (int i = 0; number != 0; i++) {
    array[i] = number % 10;
    number = number / 10;
    count++;
    }
   }
    while(count < 13); 
    if ((array[count - 1] == 4) || (array[count - 1] == 5) ||     (array[count-  1] == 3 && array[count - 2] == 7)){
        if (isValid(array) == true) {
        System.out.println("\n The Credit Card Number is Valid. ");
    } else {
        System.out.println("\n The Credit Card Number is Invalid. ");
    }
    } else{
      System.out.println("\n The Credit Card Number is Invalid. ");
    }
    in.close();
}

public static boolean isValid(long[] array) {
    int total = sumOfDoubleEvenPlace(array) + sumOfOddPlace(array);        
    if ((total % 10 == 0)) {
      for (int i=0; i< array.length; i++){
        System.out.println(array[i]);}
        return true;
    } else {
      for (int i=0; i< array.length; i++){
        System.out.println(array[i]);}
        return false;
    }
}

public static int getDigit(int number) {
    if (number <= 9) {
        return number;
    } else {
        int firstDigit = number % 10;
        int secondDigit = (int) (number / 10);
        return firstDigit + secondDigit;
    }
}

public static int sumOfOddPlace(long[] array) {
    int result = 0;
    for (int i=0; i< array.length; i++)
    {
    while (array[i] > 0) {
        result += (int) (array[i] % 10);
        array[i] = array[i] / 100;
    }

 }
    System.out.println("\n The sum of odd place is " + result);
    return result;
}

public static int sumOfDoubleEvenPlace(long[] array) {
    int result = 0;
    long temp = 0;
    for (int i=0; i< array.length; i++){
    while (array[i] > 0) {
        temp = array[i] % 100;
        result += getDigit((int) (temp / 10) * 2);
        array[i] = array[i] / 100;
       }
      }


     System.out.println("\n The sum of double even place is " + result);
    return result;
       }
     }

---

I also found a solution with less lines of logic. I know you're probably searching for an OO approach with functions, building from this could be of some help.

Similar question regarding error in Luhn algorithm logic:

Check Credit Card Validity using Luhn Algorithm

Link to shorter solution:

https://code.google.com/p/gnuc-credit-card-checker/source/browse/trunk/CCCheckerPro/src/com/gnuc/java/ccc/Luhn.java

And here I tested the solution with real CC numbers:

 public class CreditCardValidation{

    public static boolean Check(String ccNumber)

    {

            int sum = 0;
            boolean alternate = false;
            for (int i = ccNumber.length() - 1; i >= 0; i--)
            {
                    int n = Integer.parseInt(ccNumber.substring(i, i + 1));
                    if (alternate)
                    {
                            n *= 2;
                            if (n > 9)
                            {
                                    n = (n % 10) + 1;
                            }
                    }
                    sum += n;
                    alternate = !alternate;
            }
            return (sum % 10 == 0);
    }

    public static void main(String[] args){
        //String num = "REPLACE WITH VALID NUMBER"; //Valid
        String num = REPLACE WITH INVALID NUMBER; //Invalid
               num = num.trim();
        if(Check(num)){
            System.out.println("Valid");
        }
        else
            System.out.println("Invalid");
        //Check();
    }

}