Palindrome checker program

Question

I've been studying Java for some days and I'm just now starting to make some simple programs, I'd like to know if there's any 'noob' mistakes or habits that I could avoid.

In other words: how could my code be more streamlined to Java?

My code:

import java.util.Scanner;

public class Main
{
    private static String reverse_string(String my_string)
    {
        String reversed_string = "";
        for(int j = my_string.length() - 1; j >= 0; j--)
        {
            reversed_string = reversed_string + my_string.charAt(j);
        }
        return reversed_string;
    }

    public static void main(String[] args)
    {
        System.out.print("Insert a 'String': ");

        Scanner input = new Scanner(System.in);
        String user_string = input.nextLine().toLowerCase().replace(" ", "");

        if (user_string.equals(reverse_string(user_string)))
            {
                System.out.println("It is a palindrome.");
            } else
            {
                System.out.println("It is not a palindrome.");
            }
    }
}

I believe there are already other alternatives to my reverse_string function, hence the 'reinventing the wheel' tag.

Answer 1

To answer your comment, yes there are equivalent of PEP8 in java. I suggest checkstyle, this plugin works in lots of IDE / text editors and can be configured with your code style.

Code review

reverse_string method

1. In java, we try to use the upper-case version of Snake case only on constants / enums. For the methods and variables, I suggest that you use the Camel case style.

Before

private static String reverse_string(String my_string) 
{
   //[..]
}

After

private static String reverseString(String myString) 
{
   //[..]
}

2. When concatenating String in a loop, it's generally recommended to use a java.lang.StringBuilder to gain performance and take fewer operations.

private static String reverseString(String myString) 
{
   StringBuilder reversedString = new StringBuilder();

   for (int j = myString.length() - 1; j >= 0; j--) 
   {
      reversedString.append(myString.charAt(j));
   }

   return reversedString.toString();
}

If you want not to use it, then I suggest the += operator instead of the +; it will give the same result and make the code shorter.

Other observations

The code was missing a bit of formatting, but nothing too important, I sugest that you pick a formatter for your style (Horstmann style) depending on your IDE / text editor.

Refactored code

private static String reverseString(String myString)
{
   String reversedString = "";

   for (int j = myString.length() - 1; j >= 0; j--) 
   {
      reversedString += myString.charAt(j);
   }

   return reversedString;
}

public static void main(String[] args) 
{
   System.out.print("Insert a 'String': ");

   Scanner input = new Scanner(System.in);
   String userString = input.nextLine().toLowerCase().replace(" ", "");

   if (userString.equals(reverseString(userString))) 
   {
      System.out.println("It is a palindrome.");
   } 
   else 
   {
      System.out.println("It is not a palindrome.");
   }
}

Answer 2

In java is the preferred way of curly brace placing on the same line like you can see in code snippets in Oracles Style Guide.

---

reverse_string has a type embedded in its name which leads to a code smell.

The disadvantage is that if you want to change the type of the parameter you have to change the method name too.

Additional the invocation looks messy and redundant compared to a method name without the embedded type:

if (user_string.equals(reverse_string(user_string)))

compared to

if (user_string.equals(reverse(user_string)))

---

Code can be descriptive by choosing good variable names.

Scanner input = new Scanner(System.in);
String user_string = input.nextLine().toLowerCase().replace(" ", "");

I think input fits better than user_string to describe the input of a user:

Scanner scanner = new Scanner(System.in);
String input = scanner.nextLine().toLowerCase().replace(" ", "");

if (input.equals(reverse(input))) { /*...*/ }

---

if (user_string.equals(reverse_string(user_string)))
{
  System.out.println("It is a palindrome.");
} else
{
  System.out.println("It is not a palindrome.");
}

The two System.out.println are a bit like a duplication. You could save the message you want to print in a variable and then print it once to the console:

String message;
if (user_string.equals(reverse_string(user_string))) {
  message = "It is a palindrome.";
} else {
  message = "It is not a palindrome.";
}

System.out.println(message);

Or even shorter with the ternary operator:

String message = user_string.equals(reverse_string(user_string)) ? "It is a palindrome." : "It is not a palindrome.";
System.out.println(message);

---

I know that this a small script but I would like to introduce to think in objects:

public class Main {

  private static final WhiteSpaceFreeScanner scanner = new WhiteSpaceFreeScanner(new Scanner(System.in));

  public static void main(String[] args) {
    final Input input = scanner.nextLine();

    String message = input.isPalindrome() ? "It is a palindrome." : "It is not a palindrome.";
    System.out.println(message);
  }
}

class WhiteSpaceFreeScanner {

  private final Scanner scanner;

  CustomScanner(Scanner scanner) {
    this.scanner = scanner;
  }

  Input nextLine() {
    String input = scanner.nextLine().toLowerCase().replace(" ", "");
    return new Input(input);
  }

}

class Input {
  private final String value;

  public Input(String value) {
    this.value = value;
  }

  Input reversed() { /* ... */ }

  boolean isPalindrome() {
    return this.equals(reversed());
  }

  @Override
  public boolean equals(Object o) { /* ... */ }

  @Override
  public int hashCode() { /* ... */ }
}

Answer 3

Doi9t's answer is very good, but even with their improvements, there is still a problem: your code does not produce the correct answer in all cases.

Java strings use UTF-16 encoding. This means that a Java char is not large enough to store all Unicode characters. Instead some characters (for example, 😂) are stored as a pair of chars, and reversing the pair (as your code does) will result in nonsense data. See this documentation for more details.

Fortunately, the way UTF-16 is defined, chars that are surrogates (half-characters) have a completely separate range of values from chars that are Unicode characters by themselves. This means it is possible to test each char individually to see if it is a surrogate, and then have special handling to preserve the pairs.

import java.lang.Character;
import java.lang.StringBuilder;
import java.util.Scanner;

<...>

    private static String reverseString(String myString) {
       StringBuilder reversedString = new StringBuilder();

       for (int j = myString.length() - 1; j >= 0; j--) {
          char c = myString.charAt(j);
          if (Character.isLowSurrogate(c)) {
              j--;
              reversedString.append(myString.charAt(j));
          }
          reversedString.append(c);
       }

       return reversedString.toString();
    }

If you really wanted to re-invent the wheel, I think Character.isLowSurrogate(c) could be replaced with c >= '\uDC00' && c <= '\uDFFF', though I have not personally tested this.

---

As Peter Cordes pointed out in a comment, we do not even need to reverse the string in order to detect a palindrome. Instead we can examine the input string in place, comparing the first character to the last, the second to the next-to-last, etc., until we reach the middle. This may be more performant.

We need special handling for 2-char characters in this case as well; fortunately, the String class has methods for pulling code point values instead of pulling char values directly.

• codePointAt(int index) behaves similarly to charAt(int index) in most cases, but if the char at the given index is the first half of a surrogate pair, it will return the full value of the pair.
• codePointBefore(int index) approaches the problem from the other end; if the char before the given index is the last half of a surrogate pair, it will return the full value of the pair.

    private static boolean isPalindrome(String myString) {
        int len = myString.length();
        for (int i = 0; i < len / 2; ++i) {
            int frontHalfCharacter = myString.codePointAt(i);
            int backHalfCharacter = myString.codePointBefore(len - i);
            if (frontHalfCharacter != backHalfCharacter) {
                return false;
            }
            if (Character.isSupplementaryCodePoint(frontHalfCharacter)) {  // i.e. if this is a 2-char character
                i++;
            }
        }
        return true;    
    }

---

And while we're on the topic of Unicode, you should of course read The Absolute Minimum Every Software Developer Absolutely, Positively Must Know About Unicode and Character Sets (No Excuses!), if you haven't already.

Answer 4

Do you actually need to create the reversed string. Why not just go over the string from both directions.

  String palindrome = "radar";
  isPalindrome(palindrome, 0, palindrome.length() -1);     

  // Could be converted to loop if wanted.
  private static boolean isPalindrome(String candidate, int startIndex, int endIndex) {
    if(startIndex >= endIndex) {
      // Has passed each other or pointing towards same character.
      return true;
    }
    else if (candidate.charAt(startIndex) == candidate.charAt(endIndex)) {
      return isPalindrome(candidate, startIndex + 1, endIndex - 1);
    }

    return false;
  }

Answer 5

For reverse_string, it would probably be most efficient to create an array of chars and then construct a string from it. Note that I have not tested this code.

private static String reverse_string(String my_string)
{
    char[] chars = new char[my_string.length()];
    for(int i = 0; i < chars.length; ++I)
    {
        chars[i] = my_string.charAt(chars.length - 1 - i); // The two indices can be swapped
    }
    return new String(chars);
}