6
\$\begingroup\$

I wrote a program for a college assignment, and I'd like to receive some pointers on making my code a little more efficient.

The program should ask the end user to enter an integer. Use the isPalidrome method to invoke the reverse method, and report whether the integer is a palindrome. Any one-digit integer or negative integer should be rejected with the specific error message (negative or one digit), and then the program should ask the user to re-enter the integer. Hint: To reverse the digits of a number, try this routine:

int result = 0;                           
while (number != 0) {                   // e.g. number = 123
                                        // Iteration 1        Iteration 2    Iteration 3 
  int remainder = number % 10;          // remainder = 3      remainder = 2  remainder = 1 
  result = result * 10 + remainder;     // result = 3         result = 32    result = 321
  number = number / 10;                 // number = 12        number = 1     number = 0
}                                       // result contains the reverse of number

My solution:

package lab07;

import java.util.Scanner;
public class Lab07 {

public static int reverse(int number){

       int result = 0;
       while (number !=0){
           int remainder = number % 10;
           result = result * 10 + remainder;
           number = number / 10;
       }
       return result;
   }
   public static boolean isPalindrome(int input){

   int Palindrome = reverse(input);

   if (Palindrome == input){

       return true;          
   }
   else
       return false;
   }
    public static void main(String[] args) {

   int integer = 0;
   Scanner input = new Scanner(System.in);      
   System.out.print("Enter a positive, multi-digit integer: ");
   integer = input.nextInt();
   while (integer <= 9 && integer > 0)
   {
        System.out.println(integer + " is a single digit. Please re-enter another integer: ");
        integer = input.nextInt();
                if (isPalindrome(integer) && (integer > 0 && integer > 9))
           {
                System.out.println(integer + " is a palindrome");
                return;
           }
           else if (!isPalindrome(integer) && (integer > 0 && integer > 9))
           {
                System.out.println(integer + " is not a palindrome");
                return;
           }

   }
   while (integer < 0)
   {
       System.out.println(integer + " is negative. Please re-enter another integer: ");
       integer = input.nextInt();
                if (isPalindrome(integer) && (integer > 0 && integer > 9))
            {
                 System.out.println(integer + " is a palindrome");
                 return;
            }
            else if (!isPalindrome(integer) && (integer > 0 && integer > 9))
            {
                 System.out.println(integer + " is not a palindrome");
                 return;
            }
                 while (integer <= 9 && integer > 0)
                        {
                             System.out.println(integer + " is a single digit. Please re-enter another integer: ");
                             integer = input.nextInt();
                                     if (isPalindrome(integer) && (integer > 0 && integer > 9))
                                {
                                     System.out.println(integer + " is a palindrome");
                                     return;
                                }
                                else if (!isPalindrome(integer) && (integer > 0 && integer > 9))
                                {
                                     System.out.println(integer + " is not a palindrome");
                                     return;
                                }
                        }                   
   }
   while (integer > 9){
                if (isPalindrome(integer) && (integer > 0 && integer > 9))
                {
                     System.out.println(integer + " is a palindrome");
                     return;
                }
                else if (!isPalindrome(integer) && (integer > 0 && integer > 9))
                {
                     System.out.println(integer + " is not a palindrome");
                     return;
                }
   }

}

   }
\$\endgroup\$
1
  • \$\begingroup\$ Am I going to be the one who says that all single digit numbers are palindromes? A special case I'll grant but if ABA is a palindrome what is wrong with B as a palindrome? I think the empty string is a palindrome. But don't listen to me. \$\endgroup\$
    – user59064
    Commented Feb 26, 2015 at 15:10

4 Answers 4

Reset to default
5
\$\begingroup\$

Apart from the other comments about how to calculate the input, I would also suggest that your main method is buggy, and not very 'pretty'. One the the tricks in programming that Java programmers seem to forget, is the do-while loop (note, C programmers use this all the time, and Java programmers do all sorts of crazy things to avoid it ...! ).

Consider the an altered main method

  • renamed Scanner to be scanner
  • renamed integer to be input.
  • use do-while for input validation
  • use 'simpler' printf for output
public static void main(String[] args) {

    int input = 0;
    Scanner scanner = new Scanner(System.in);      
    boolean ok = false;
    do {
        System.out.print("Enter a positive, multi-digit integer: ");
        input = scanner.nextInt();
        if (input < 0) {
            System.out.println(input + " is negative. Please re-enter another integer: ");
        } else if (input <= 9) {
            System.out.println(input + " is a single digit. Please re-enter another integer: ");
        } else {
            ok = true;
        }
    } while (!ok);

    System.out.printf("%d %s a palindrome\n", input, isPalindrome(input) ? "is" : "is not");
    scanner.close();

}

I should add that having 'return' in a main method is unconventional.... not 'wrong' but it probably means you are doing something weird...

\$\endgroup\$
4
  • \$\begingroup\$ How do you code isPalindrom(Integer) method? - Why are you transforming the input in a int? keep it as String : (s==null || s.startwith('-') || s.length <2 ) --> false; is also a good test to protect StringBuilder method \$\endgroup\$
    – cl-r
    Commented Nov 5, 2013 at 12:56
  • \$\begingroup\$ Other people have already suggested answers for those questions. My notes are purely on the main method. As for your specific questions, I have used the OP's isPalindome() method and I think it is a better method than using String (since it's easier to validate the int). Also, I have just copy/pasted the OP's code and moved it, keeping as much of his 'style' as I can. It is not my intention to criticize or compete against other answers here \$\endgroup\$
    – rolfl
    Commented Nov 5, 2013 at 13:05
  • 1
    \$\begingroup\$ I wouldn't suggest using do-while here. Not that do-while is bad as such, but having an ok flag isn't any better. To me the approach by @raistlinthewizard (using continue) is way more elegant. \$\endgroup\$
    – Ihor Kaharlichenko
    Commented Dec 17, 2013 at 7:16
  • \$\begingroup\$ The initial prompt belongs before the loop. \$\endgroup\$
    – 200_success
    Commented Dec 17, 2013 at 10:50
5
\$\begingroup\$

First: There is a wrong template in your code:

 if (isPalindrome(integer) && (integer > 0 && integer > 9))
       {
            System.out.println(integer + " is a palindrome");
            return;
       }
       else if (!isPalindrome(integer) && (integer > 0 && integer > 9))
       {
            System.out.println(integer + " is not a palindrome");
            return;
       }
  1. (integer > 0 && integer > 9) is equals to integer > 9
  2. The isPalindrome(integer) function used 2 times, but only need to invoke it 1 times per iteration.

Modified code:

 final boolean inputIsPalindrome = isPalindrome(integer);
 if (inputIsPalindrome && integer > 9))
   {
        System.out.println(integer + " is a palindrome");
        return;
   }
   else if (!inputIsPalindrome && integer > 9)
   {
        System.out.println(integer + " is not a palindrome");
        return;
   }

Here 2 problemes still there:

  1. checking double times the integer's value.
  2. Here you need only "if {} else {}" construct, because if the first true, the second is false.

So the re-modified code:

 if (integer > 9) {
   if (isPalindrome(integer)) {
        System.out.println(integer + " is a palindrome");
        // don't need "else" if returning here
        return;
   }

   System.out.println(integer + " is not a palindrome");
   return;
 }

More:

  1. You have a lot of loops to get the user input, but you only need one.
  2. In Java variables use CamelCase beginning with lower case character.
  3. The variable names have to tell more about itself.
  4. Handling input with wrong type is necessary.
  5. In this case I prefer reverse string instead of compute the reverse of the integer.

I've wrote your program here:

package lab07;

import java.util.Scanner;

public class Lab07 {

public static void main(String[] args) {

    System.out.print("Enter a positive, multi-digit integer: ");
    final String inputIntegerAsString = readInputIntegerAsString();

    if (isPalindrome(inputIntegerAsString)) {
        System.out.println(inputIntegerAsString + " is a palindrome");
        return;
    }

    System.out.println(inputIntegerAsString + " is not a palindrome");
}

private static String readInputIntegerAsString() {
    try (final Scanner scanner = new Scanner(System.in)) {
        // Instead of "true" you can use an exit condition, but in
        // this simple case I don't think to care about that.
        while (true) {
            final String line = scanner.nextLine();

            int inputInteger = 0;
            try {
                inputInteger = Integer.parseInt(line);
            } catch (final NumberFormatException e) {
                System.out.println("Invalid input. Please re-enter another integer: ");
                continue;
            }

            if (inputInteger < 10) {
                if (inputInteger < 0) {
                    System.out.println(inputInteger + " is negative. Please re-enter another integer: ");
                    continue;
                }

                System.out.println(inputInteger + " is a single digit. Please re-enter another integer: ");
                continue;
            }
            return line;
        }
    }
}

private static boolean isPalindrome(final String originalInputAsString) {
    final String reversedString = reverseString(originalInputAsString);

    return reversedString.equals(originalInputAsString);
}

private static String reverseString(final String originalString) {
    final StringBuilder stringBuilder = new StringBuilder(originalString);
    final String reversedString = stringBuilder.reverse().toString();

    return reversedString;
}
}
\$\endgroup\$
1
  • \$\begingroup\$ Make inputInteger final and do not initialize it before try clause. \$\endgroup\$
    – Ihor Kaharlichenko
    Commented Dec 17, 2013 at 7:25
5
\$\begingroup\$

Adding a second answer here because this answer is directly targetting your actual question: "How to make the palindrome program more efficient". ince the fastest answer I tested has not been mentioned yet in this review, I figure it is worth outlining here...

Personally I feel that the int approach to dealing with it is more intuitive, but I set out to prove this, and I was suprised, so, here is my test program....

First, it generates a million integer values stored as Strings in a char[] array. It then repeatedly 'scans' those values and checks each value to see whether it is a palindrome. In the spirit of the initial program, the test here is really whether converting the value to an int and then doing numberical manipulation is better than keeping the value as a String, and doing String comparisons..... I chose two different mechanisms for String compares....

Bottom line is that keeping the values as Strings is about 50% faster... huh. Half the time.

The most efficient method is:

public static boolean isPalindromeChar(final String input){
    int front = 0;
    int rear = input.length() - 1;
    while (front < rear) {
        if (input.charAt(front++) != input.charAt(rear--)) {
            return false;
        }
    }
    return true;
}

which is (on my machine) about 15% faster than:

public static boolean isPalindromeSBuilder(final String input){
    return input.equals(new StringBuilder(input).reverse().toString());
}

Doing Integer-based processing is significantly slower, probably because there's a lot of overhead in the Scanner.nextInt() method.

Here is my full code, but my sunmmary output is:

Average integer took 1.0457 seconds,
average string took 0.7060 seconds,
average char took 0.6139 seconds

Obviously, to benchmark this process I had to take some liberties, and the code is not a great solution to the original problem, but, as for the most efficient way to calculate palindromes from a Scanner, it is clearly to keep the valeus as Strings, and to do char-based comparisons.....

import java.io.CharArrayReader;
import java.io.CharArrayWriter;
import java.io.IOException;
import java.util.Random;
import java.util.Scanner;

public class Lab07 {

    public static int reverse(int number){

        int result = 0;
        while (number !=0){
            int remainder = number % 10;
            result = result * 10 + remainder;
            number = number / 10;
        }
        return result;
    }
    public static boolean isPalindromeInt(final int input){
        return input == reverse(input);
    }

    public static boolean isPalindromeSBuilder(final String input){
        return input.equals(new StringBuilder(input).reverse().toString());
    }

    public static boolean isPalindromeChar(final String input){
        int front = 0; int rear = input.length() - 1;
        while (front < rear) {
            if (input.charAt(front++) != input.charAt(rear--)) {
                return false;
            }
        }
        return true;
    }

    private static final void palindromeInteger(char[] data, int count) {
        CharArrayReader reader = new CharArrayReader(data);
        int palcount = 0;
        Scanner scanner = new Scanner(reader);
        while (count > 0) {
            int input = 0;
            boolean ok = false;
            do {
                //System.out.print("Enter a positive, multi-digit integer: ");
                input = scanner.nextInt();
                count--;
                if (input < 0) {
                    System.out.println(input + " is negative. Please re-enter another integer: ");
                } else if (input <= 9) {
                    System.out.println(input + " is a single digit. Please re-enter another integer: ");
                } else {
                    ok = true;
                }
            } while (!ok);

            if (isPalindromeInt(input)) {
                palcount++;
            }
        }
        System.out.printf("%d palindromes\n", palcount);
        scanner.close();
    }

    private static final void palindromeString(char[] data, int count) {
        CharArrayReader reader = new CharArrayReader(data);
        int palcount = 0;
        Scanner scanner = new Scanner(reader);
        while (count > 0) {
            String input = null;
            boolean ok = false;
            do {
                //System.out.print("Enter a positive, multi-digit integer: ");
                input = scanner.next();
                count--;
                if (input.startsWith("-")) {
                    System.out.println(input + " is negative. Please re-enter another integer: ");
                } else if (input.length() < 2) {
                    System.out.println(input + " is a single digit. Please re-enter another integer: ");
                } else {
                    ok = true;
                }
            } while (!ok);

            if (isPalindromeSBuilder(input)) {
                palcount++;
            }
        }
        System.out.printf("%d palindromes\n", palcount);
        scanner.close();
    }

    private static final void palindromeChar(char[] data, int count) {
        CharArrayReader reader = new CharArrayReader(data);
        int palcount = 0;
        Scanner scanner = new Scanner(reader);
        while (count > 0) {
            String input = null;
            boolean ok = false;
            do {
                //System.out.print("Enter a positive, multi-digit integer: ");
                input = scanner.next();
                count--;
                if (input.startsWith("-")) {
                    System.out.println(input + " is negative. Please re-enter another integer: ");
                } else if (input.length() < 2) {
                    System.out.println(input + " is a single digit. Please re-enter another integer: ");
                } else {
                    ok = true;
                }
            } while (!ok);

            if (isPalindromeChar(input)) {
                palcount++;
            }
        }
        System.out.printf("%d palindromes\n", palcount);
        scanner.close();
    }

    public static void main(String[] args) throws IOException {

        System.out.println("Generating values");

        CharArrayWriter caw = new CharArrayWriter();
        Random rand = new Random();
        final int numbers = 1000000;
        for (int i = 0; i < numbers; i++) {
            caw.write(rand.nextInt(numbers) + "\n");
        }
        caw.close();
        final char[] data = caw.toCharArray();

        System.out.println("Generated " + numbers + " values");

        long inttimes = 0L;
        long strtimes = 0L;
        long chrtimes = 0L;
        for (int loop = 0; loop < 11; loop++) {
            long intstart = System.nanoTime();
            palindromeInteger(data, numbers);
            if (loop > 0) {
                inttimes += System.nanoTime() - intstart;
            }
            long strstart = System.nanoTime();
            palindromeString(data, numbers);
            if (loop > 0) {
                strtimes += System.nanoTime() - strstart;
            }
            long chrstart = System.nanoTime();
            palindromeChar(data, numbers);
            if (loop > 0) {
                chrtimes += System.nanoTime() - chrstart;
            }
        }

        System.out.printf("Average %s took %.4f seconds, average %s took %.4f seconds, average %s took %.4f seconds\n",
                "integer", (inttimes / 10) / 1000000000.0,
                "string", (strtimes / 10) / 1000000000.0,
                "char", (chrtimes/ 10) / 1000000000.0);


    }

}
\$\endgroup\$
3
\$\begingroup\$ 
if (isPalindrome(integer) && (integer > 0 && integer > 9))
                    {
                         System.out.println(integer + " is a palindrome");
                         return;
                }

This piece should be moved to a separed method, for example:

private void checkInt(int value){
        if(value > 0 && value > 9){
        if (isPalindrome(value))
        {
            System.out.println(value + " is a palindrome");
        }
        else if (!isPalindrome(value) && (value > 0 && value > 9))
        {
            System.out.println(value + " is not a palindrome");
        }
        return;
    }

if (Palindrome == input){
           return true;}   
else{
           return false;}

This could be reduced to "return Palindrome == input"

Do not ever give variables such name as "integer","string" or any other class name.

Do not ever capitalize variable name: "int Palindrome"

\$\endgroup\$
1
  • \$\begingroup\$ Your code is longer than necessary. The condition value > 0 && value > 9 can be simplified to value > 9, you should not call isPalindrome twice. \$\endgroup\$
    – Roland Illig
    Commented Dec 17, 2013 at 5:51

Not the answer you're looking for? Browse other questions tagged or ask your own question.