3
\$\begingroup\$

I've just started learning Java and went to compare my finished product to some others on Stack Overflow. Is there a reason why mine is "simple" and the others seem ridiculously hard to even understand (for me)? Link to the other palindrome checker.

Can you just break it down for me (the good/bad/ugly of my code)? Constructive criticism would be great as I'm very good at memorizing the books I use to learn Java. I just have a hard time implementing them and it's always good to have a second opinion.

public class Checker
{
    public static void main(String [] args) {
        String original = "hannah";
        String reversed = original;
        for (String part : original.split(" ")) {
            System.out.println(original);
            System.out.println(new StringBuilder(part).reverse().toString());
        }
        System.out.print(original.equals(new StringBuilder(reversed).reverse().toString()));
    }
}
\$\endgroup\$
  • \$\begingroup\$ Can you just break it down for me. The good/bad/ugly of my code? Constructive criticism would be great as I'm very good at memorizing the books I use to learn java I just have a hard time implementing them and its always good to have a second opinion. :) \$\endgroup\$ – IDV Sep 22 '13 at 4:17
  • \$\begingroup\$ Ah, okay. That's more clear. :-) I'm not a Java person, so someone else will have to look at this. \$\endgroup\$ – Jamal Sep 22 '13 at 4:20
  • 1
    \$\begingroup\$ In opposite to the 'other palindrome checker' you create a new reversed String and than compare them. The 'other palindrome checker' does not creates any new String and only reads each character once. Therefore the 'other palindrome checker' is likely to be faster and needs less memory. \$\endgroup\$ – MrSmith42 Sep 22 '13 at 7:52
  • \$\begingroup\$ I just want to point out that your solution is also the one that got the most votes in the stackoverflow post you linked above. It's always a good idea to read more than just the selected answer when consulting stackoverflow. \$\endgroup\$ – toto2 Sep 22 '13 at 14:58
  • \$\begingroup\$ Yea, I did see that selected answer, however I was curious as to what made mine different than the original question. Like I said, I'm very new to this and was just curious. :) \$\endgroup\$ – IDV Sep 23 '13 at 0:43
4
\$\begingroup\$

The most misleading part of the code is this: String reversed = original;reversed is not reversed at all! Let's get rid of that confusion right away. Also note a few minor points…

public class Checker
{
    public static void main(String[] args) {
        // Consider using args instead of hard-coding your test case
        String original = "hannah";
        for (String part : original.split(" ")) {
            // I think you meant to print part instead of original here...
            System.out.println(part);
            // System.out.println() implicitly calls .toString() on its argument
            System.out.println(new StringBuilder(part).reverse());
        }
        System.out.print(original.equals(new StringBuilder(original).reverse().toString()));
    }
}

Get in the habit of splitting meaningful chunks of work into functions whenever possible.

public class Checker
{
    private static String reverse(String s) {
        return new StringBuilder(s).reverse().toString();
    }

    public static boolean isPalindrome(String original) {
        return original.equals(reverse(original));
    }

    public static void main(String[] args) {
        String original = "hannah";
        for (String word : original.split(" ")) {
            System.out.println(word);
            System.out.println(reverse(word));
        }
        System.out.print(isPalindrome(original));
    }
}

That's more readable now, isn't it?

\$\endgroup\$
  • \$\begingroup\$ Thank you 200_success. I was just going by the default "public static void main(String [] args) {" that shows in my book as this was kind of straying away from what the book is teaching. I know farther on it goes into using a "tester" so you can test your code easier. \$\endgroup\$ – IDV Sep 23 '13 at 0:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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