patternjavaModerate
Palindrome Checker in Java
Viewed 0 times
palindromecheckerjava
Problem
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;
}
}Solution
Firstly, this is something I see a distressing amount in beginner Java programmers:
There's a method for checking if two Strings are equal. It's called
If you wanna ignore case, you could use
Secondly, a formatting tip: Except in method declarations or calls, always put a space before
becomes
In your
With those issues fixed, this is what the method looks like:
(Note that I changed what would have been
The whole function could be replaced with
You use
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
In that snippet above, I also removed
With all this, your
We could clean it up a little more by moving your
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));
}
removeSpace(word).compareTo(removeSpace(reversed)) == 0There'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 aStringBuilderandappendto it.
String#charAt()returns achar, which can be compared with==; I'm not sure why you first turn it into aString, then usecompareTo(which, again, should beequals, 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 meansWith 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));
}
Code Snippets
removeSpace(word).compareTo(removeSpace(reversed)) == 0removeSpace(word).equals(removeSpace(reversed))removeSpace(word).equalsIgnoreCase(removeSpace(reversed))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();
}word = removeSpaces(in.nextLine());
// ...
if(word.equals(reversed)) { // Note that I'm not using compareTo here
// etc.Context
StackExchange Code Review Q#92796, answer score: 19
Revisions (0)
No revisions yet.