1

I'm very new to programming. Can you tell me why this outputs:

string index out of range: -1

class Palindrome {

    public static String reverse(String s) {
        if (s == "") {
            return s;
        }
        else {
            return reverse(s.substring(1,s.length())) + s.charAt(0);
        }
    }

    public static void main(String args[]) {
        System.out.print(reverse("galskjdf"));
    }
}   
takendarkk
  • 3,347
  • 8
  • 25
  • 37
user252017
  • 19
  • 4
  • @ZouZou there is no String comparison in this code, only substring, i don't see how this question is applicable – Absurd-Mind Mar 28 '14 at 14:52
  • 1
    @Absurd-Mind How about `if (s == "")`? – Alexis C. Mar 28 '14 at 14:52
  • lol, not a single day without this duplicate :-) I barely know Java but it seems like a design flaw to me, although I understand why it's not normal so many people fall on this. – Laurent S. Mar 28 '14 at 14:52
  • 1
    @Bartdude This is not a design flaw. People need to know basic Java before programming, just as you are ought to know about pointers when dealing with C and variants. – skiwi Mar 28 '14 at 14:53
  • 3
    @turbo The `endIndex` is exclusive. So `s.length()` is valid. – Alexis C. Mar 28 '14 at 14:54
  • @ZouZou ah, I always get that mixed up about substring – turbo Mar 28 '14 at 14:55
  • @skiwi > I know, all languages have their specificities, but from my non-java point of view I find it very un-intuitive to compare strings and numbers with such a different syntax... All the languages I've worked with, although representing a very small proportion of existing languages, made it quite similar to compare strings or numbers. Well at least if I'm ever into Java I won't make this error :-) – Laurent S. Mar 28 '14 at 15:08

3 Answers3

1

Because you are doing string comparison in a wrong way => recursion doesn't stop at the valid point. Replace this:

s == ""

with this:

"".equals(s)

EDIT Well, looks like proof is needed, here it is: http://ideone.com/TWm5U8

nikis
  • 11,166
  • 2
  • 35
  • 45
  • although this is not untrue, this doesn't answer the question; The error message doesn't come from this... – Laurent S. Mar 28 '14 at 14:53
  • 1
    @Bartdude Well I tried the correction on the given code and it works ;) – Michael Laffargue Mar 28 '14 at 14:55
  • @Bartdude you are wrong, proof http://ideone.com/TWm5U8 – nikis Mar 28 '14 at 14:55
  • 1
    @Bartdude it's not where the error comes from, but it causes an invalid string to reach the else statement – turbo Mar 28 '14 at 14:58
  • @turbo why do you think it's not the source of error? =) – nikis Mar 28 '14 at 15:01
  • @nikis Bartdude said "the error message doesn't come from this" in which case he is _technically_ right, but it's due to an invalid input. – turbo Mar 28 '14 at 15:07
  • I admit I was wrong. I will switch the "-1" to "+1" ... but I would prefer the answer to mention the actual cause of the error more precisely so that if the OP ever get the same kind of error, he doesn't look for a bad comparison somwhere, but for a wrong substring index somewhere... Actually I can't change my vote without an edit... – Laurent S. Mar 28 '14 at 15:10
1

Use

class Palindrome {
  public static String reverse(String s) {
    if ("".equals(s)) {
      return s;
    } else {
      return reverse(s.substring(1,s.length())) + s.charAt(0);
  }
}

  public static void main(String args[]) {
    System.out.print(reverse("galskjdf"));
  }
}   

See How do I compare strings in Java? and Reverse a string in Java might also be helpful (even though you probably did the above code as a programming exercise, it doesn't hurt to know that usually one would use already existing tools for this kind of thing).

The reason for using

"".equals(s)

instead of

s.equals("")

is as follows: In case s is null, the latter will throw a NullPointerException (because you try to call a method on null), whereas the former still works as you call a method on a "proper" string, see also Gracefully avoiding NullPointerException in Java However, I remember also some people criticizing this approach as this might make you miss the fact that s is null when it shouldn't be (in which case you should explicitly check handle that case).

Community
  • 1
  • 1
godfatherofpolka
  • 1,645
  • 1
  • 11
  • 24
0

Use this function :

public static String reverse(String s) {
    if (s == null || s.length() <= 1) {
        return s;
    } else {
        return reverse(s.substring(1)) + s.charAt(0);
    }
}
Remi878
  • 341
  • 1
  • 6