Palindrome Checker in Java

Question

My objective is to take a string as input and determine if it is a palindrome. I'm wondering how I could improve this code (efficiency, bugs, etc). I'm relatively new to Java so I'm expecting that my code will contain some errors.

import java.util.Scanner;

    /**
     * Created by Adam on 6/5/2015.
     */
    public class Reversal {
        public static void main(String args[]) {
            String word = "";
            String reversed = "";
            Scanner in = new Scanner(System.in);
            System.out.println("Please enter the string: ");
            word = in.nextLine();
            int length = word.length();
            for(int i = length; i > 0; i--) {
                reversed += word.charAt(i-1);
            }
            if(removeSpace(word).compareTo(removeSpace(reversed)) == 0) {
                System.out.println("The string " + word + " is a palindrome");
            }
            else {
                System.out.println("The string " + word + " is not a palindrome");
            }
        }

        private static String removeSpace(String aword) {
         String buffer = "";
            for(int i = 0; i < aword.length(); i++)   {
             if(!(String.valueOf(aword.charAt(i)).compareTo(" ") == 0)) {
                 buffer += aword.charAt(i);
             }
         }
            return buffer;
        }
    }

Answer 1

Firstly, this is something I see a distressing amount in beginner Java programmers:

removeSpace(word).compareTo(removeSpace(reversed)) == 0

There's a method for checking if two Strings are equal. It's called equals. It'd be used like this:

removeSpace(word).equals(removeSpace(reversed))

If you wanna ignore case, you could use equalsIgnoreCase:

removeSpace(word).equalsIgnoreCase(removeSpace(reversed))

Secondly, a formatting tip: Except in method declarations or calls, always put a space before ( and a space after ). So, for example:

if(...) {

becomes

if (...) {

In your removeSpaces method:

• In general, when find that you're doing it a lot, avoid using String += String. Instead, initialize a StringBuilder and append to it.
• String#charAt() returns a char, which can be compared with ==; I'm not sure why you first turn it into a String, then use compareTo (which, again, should be equals, since you were comparing Strings for equality)

With those issues fixed, this is what the method looks like:

private static String removeSpace(String aword) {
     StringBuilder buffer = new StringBuilder();
        for (int i = 0; i < aword.length(); i++)   {
         if (aword.charAt(i) != ' ')) {
             buffer.append(aword.charAt(i));
         }
     }
     return buffer.toString();
}

(Note that I changed what would have been !(... == ...) to ... != ... and that ' ' uses single quotes.)

The whole function could be replaced with word.replaceAll(" ", ""), but that's no fun, so I'm gonna keep using yours through here. Besides, it's a good way to practice to write your own methods -- just remember, when you're writing production code, to do a thorough search of the docs first. If there's a version in the standard API, a whole gaggle of people have been optimizing it to high heaven, and it might even be implemented in native code. That alone would make it a bazillion times faster than anything you could write in pure Java.

You use removeSpaces in the comparison, which somewhat obfuscates what really happens: The String is checked for palindromity (word coined?) ignoring the spaces. It's clearer to move those method calls to when you get it (plus, this removes an extra function call, which is more efficient :D):

word = removeSpaces(in.nextLine());
// ...
if(word.equals(reversed)) { // Note that I'm not using compareTo here
// etc.

Thirdly, I'd recommend pulling the code that reverses your String into another method that takes a String argument and returns a String (and applying my earlier advice about StringBuilder):

public static String reverse(String word) {
    StringBuilder reversed = new StringBuilder(word.length);
    for(int i = word.length(); i > 0; i--) {
        reversed.append(word.charAt(i - 1));
    }
}

In that snippet above, I also removed length, since it's ultimately useless. You're only using it once, so there's no point in declaring a variable, and it's actually clearer to be able to directly see what String's length we're talking about -- you don't need to go back in the code to see what it means

With all this, your main becomes a bit clearer:

public static void main(String args[]) {
    String word = "";
    String reversed = "";
    Scanner in = new Scanner(System.in);
    System.out.println("Please enter the string: ");
    word = removeSpaces(in.nextLine());
    reversed = reverse(word);
    if (word.equals(reversed)) {
        System.out.println("The string " + word + " is a palindrome");
    }
    else {
        System.out.println("The string " + word + " is not a palindrome");
    }
}

We could clean it up a little more by moving your word and reversed declarations to where the variables are first assigned, rather than their own lines (which, incidentally, don't need the = "" because they aren't used before you assign them later):

public static void main(String args[]) {
    Scanner in = new Scanner(System.in);
    System.out.println("Please enter the string: ");
    String word = removeSpace(in.nextLine());
    String reversed = reverse(word);
    if (word.equals(reversed)) {
        System.out.println("The string " + word + " is a palindrome");
    }
    else {
        System.out.println("The string " + word + " is not a palindrome");
    }
}

This means that your whole class, all together, looks something like this:

public class Reversal {
    public static String reverse(String word) {
        StringBuilder reversed = new StringBuilder(word.length);
        for (int i = word.length(); i > 0; i--) {
            reversed.append(word.charAt(i - 1));
        }
    }

    private static String removeSpace(String aword) {
         StringBuilder buffer = new StringBuilder();
            for (int i = 0; i < aword.length(); i++)   {
             if(aword.charAt(i) != ' ')) {
                 buffer.append(aword.charAt(i));
             }
         }
         return buffer.toString();
    }

    public static void main(String args[]) {
        Scanner in = new Scanner(System.in);
        System.out.println("Please enter the string: ");
        String word = removeSpace(in.nextLine());
        String reversed = reverse(word);
        if (word.equals(reversed)) {
            System.out.println("The string " + word + " is a palindrome");
        }
        else {
            System.out.println("The string " + word + " is not a palindrome");
        }
    }
}

If you want to get rid of your custom method to remove spaces (which I would recommend doing if you ever had to remove spaces in production code):

public class Reversal {
    public static String reverse(String word) {
        StringBuilder reversed = new StringBuilder(word.length);
        for (int i = word.length(); i > 0; i--) {
            reversed.append(word.charAt(i - 1));
        }
    }

    private static String removeSpace(String aword) {
         StringBuilder buffer = new StringBuilder();
            for (int i = 0; i < aword.length(); i++)   {
             if(aword.charAt(i) != ' ')) {
                 buffer.append(aword.charAt(i));
             }
         }
         return buffer.toString();
    }

    public static void main(String args[]) {
        Scanner in = new Scanner(System.in);
        System.out.println("Please enter the string: ");
        String word = in.nextLine().replaceAll(" ", "");
        String reversed = reverse(word);
        if (word.equals(reversed)) {
            System.out.println("The string " + word + " is a palindrome");
        }
        else {
            System.out.println("The string " + word + " is not a palindrome");
        }
    }
}

And, finally, an utterly unreadable version of your code that does the same thing, which you should never use but was annoyingly fun to write so I thought I'd provide it anyway:

public static void main(String args[]) {
    String l;
    System.out.println("The string "+(l=new Scanner(System.in).nextLine())+" is"+(l.replaceAll(" ", "").equals(new StringBuilder(l).reverse().toString().replaceAll(" ", ""))?" ":" not ")+"a palindrome");
}

Answer 2

CODING STYLE

For the beginning, some coding habits you seem to have picked up could/should be changed. Declare your variables before use, not at beginning of the method (does not apply for loops, where you deliberately want to declare variable before loop). This was, and still is, widely used in C/C++. Earlier was possible to declare new variables only at beginning of the function, and it sticked with some programmers. Declaration before first use makes code more readable, because at beginning of a function you read some listed variables you do not know what they are for, and later on you see some of them used but do not already remember initial state for instance.

Insert empty lines between logic blocks, it makes code more readable and one can by a quick glance guess what is that block about and move along. It is also common to write comment for whole block after that empty line (if it is necessary).

Do not use articles in your naming. I suppose that is an article in variable "aword". You could be more descriptive about purpose of variables. Naming your variables in removeSpace to input/output instead of aword and buffer will leave no doubts about their purpose.

Naming of your method "reverseSpace" is unclear, because "space" is singular, which without javadoc or looking at the code says only one space will be removed. Correct name would be with plural "removeSpaces".

CODE

When you are doing something fairly simple, always consider looking for some one liner, which pays especially for operations over strings.

---

Reverse string can be done by

String output = new StringBuilder(input).reverse().toString();

Google: "java reverse string"

---

Spaces can be removed by

String output = input.replaceAll("\\s+","");

or if you want to remove only space and not all whitespaces

String output = input.replaceAll(" ","");

Google: "java remove whitespaces"

---

In condition for comparing of reversed with original, you used function compareTo. This is unnecessary and not exactly obvious at first glance. Much more suitable is using object method equals.

if(removeSpace(word).equals(removeSpace(reversed)))

---

Your code could be more optimal, because you unnecessarily call removeSpace twice, when you need to call it only after reading the input.

Another optimization would take away the reversal of the string altogether. You can just iterate over the elements and look at opposite index.

for(int i = 0;i<input.length();i++){
    if(input.charAt(i)!=input.charAt(input.length()-i-1)){
        return false;
    }
}
return true;

---

Hope this helps, regards.

Answer 3

A general rule about efficiency:
Appending a String to another String inside a loop is very inefficient. You're creating a lot of unneeded String objects this way:

for(int i = length; i > 0; i--) {
    reversed += word.charAt(i-1);
}

Instead, use a StringBuilder to append Strings together:

StringBuilder reversed = new StringBuilder();
for (int i = length; i > 0; i--) {
    reversed.append(word.charAt(i-1));
}

However, you don't need this loop, because StringBuilder has a reverse method. How convenient!

String reverse = new StringBuilder(word).reverse().toString();

You are also creating an unneeded String in this line:

if(!(String.valueOf(aword.charAt(i)).compareTo(" ") == 0))

You can just compare 2 characters instead of 2 Strings:

if (aword.charAt(i) != ' ')

But, again, you don't need it; the String class already has a method for it. Just replace all spaces with the empty String:

word = word.replace(" ", "");

But
...

You don't need any of this. The way you go about solving your problem is long-winded and inefficient to begin with. Think about what you're doing.
If a String is a palindrome (if you forget about spaces for a moment), then:
the 1st character is equal to the last character, and
the 2nd character is equal to the second-to-last-character,
etc.
This can be done in one for-loop: compare the first and last character and move towards the middle. You only need an extra check to skip the spaces.

private static boolean isPalindromeWithoutSpaces(string input) {
    int left = 0, right = input.length() - 1;
    while (left < right) {
        char leftChar = input.charAt(left);
        char rightChar = input.charAt(right);
        if (leftChar == ' ') {
            left++;
        } else if (rightChar == ' ') {
            right--;
        } else if (leftChar != rightChar) {
            return false;
        } else {
            left++;
            right--;
        }
    }
    return true;
}

This does not create any new objects whatsoever and only loops over the entire String just once.

Answer 4

Wouldn't recursion be better for this? I made a palindrome checker not too long ago and I use recursion too check the first and last character of the string and continue too work my way inwards.

public class Palindrome
{
   public static void main (String [] args)
   {
      String st = "radar";
      if (palindrome (st, 0, st.length() - 1)){
         System.out.println ("It is a palindrome :)");
      }
      else{
         System.out.println ("Not a palindrome :(");
      }
   }

   private static boolean palindrome (String s, int start, int end)
   {
      if (start >= end){
        return (true);
      }
      else{
        if (s.charAt(start) != s.charAt(end)){
          return (false);
        }
        else{
          return (palindrome (s, start + 1, end - 1));    
        }
      }
   }
}

As I said above this recursion takes the first and last character of the string and compares them. If they are equal it'll proceed with the second and beforelast characters. Therefore you won't have too reverse the string.

As soon as it finds a set of characters that's not the same it will immediatly return false and not check the rest of the string, which makes it more efficient.