current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite

I've coded to check whether the number is palindrome or not. In my code, both the str and string variable returns same string. But the function returns false.

Where am I missing? 

public class PalindromeDemo {
    public static void main(String[] args) {
        if(isPalindrome("aba")) {
            System.out.println("The given no is palindrome");
        }
    }

    static boolean isPalindrome(String str) {

        StringBuilder string = new StringBuilder();     
        for(int i=1; i <= str.length(); i++) {
            string.append(str.charAt(str.length()-i));
        }       

        if(string.equals(str))
            return true;

        return false;
    }

}
share|improve this question

closed as off-topic by palacsint, Brian Reichle, Jeff Vanzella, Jamal♦, Glenn Rogers Aug 14 '13 at 21:19

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must contain working code for us to review it here. For questions regarding specific problems encountered while coding, try Stack Overflow. After your code is working you can edit this question for reviewing your working code." – palacsint, Brian Reichle, Jeff Vanzella, Jamal, Glenn Rogers
If this question can be reworded to fit the rules in the help center, please edit the question.

2 Answers 2

up vote 4 down vote accepted

The issue with your code is that a StringBuilder is only equal to itself. please read: StringBuilder's toString method.

The method equals() is inherited from Object's equals method.

This adapted function will behave as expected.

static boolean isPalindrome(String str) {

    StringBuilder string = new StringBuilder();     
    for(int i=1; i <= str.length(); i++) {
        string.append(str.charAt(str.length()-i));
    }       
    if(string.toString().equals(str))
        return true;

    return false;
}
share|improve this answer
up vote 1 down vote

I would suggest the following approach:

static boolean isPalindrome(String str) {
   int stringLength = str.length();

   for(int i = 0; i < stringLength / 2; i++) {
       char expected = str.charAt(i);
       char found = str.charAt(stringLength - i - 1);

       if (!found.equals(expected)) {
           return false;
       }
   }
   return true;

}

This way, you just compare in each loop the two mirroring characters at the beginning and the end of the string. You're out of the loop as soon as they don't match. If they do, you will end up with the same amount of work done as in vegi's solution.

/!\ Please make sure you check for null, empty or 1 character long Strings!

share|improve this answer

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