CODEWITHSUNDEEP
javastringindexoutofboundsexception
Ely Eastman
Ely Eastman

Reputation: 235

Strange String out of Bounds exception

I am working on a method to test if a phrase is a palindrome. The method takes a string and checks if the first and last letters of that string are the same. If so, it takes away those letters and then checks the string again until there is only one (or zero) letter left using a recursive function. Currently 

String start = s.substring(0, 1);
String end = s.substring(s.length() - 1, s.length());

takes the first and last letter of each string; however, if I use a two letter string I get a String Index Out of Bounds Error. I thought changing the index to 0,0 would help but I later realized that doesn't even grab the first letter. There is no issue for a string that is 1 or more than 2 characters long. I'll put the rest of the method below if y'all need it, but currently the program doesn't even run that far with a two letter string.

public static boolean palindromeTester(String s){
       s = justLetters(s); //calls a method that removes all spaces, punctuation
       String start = s.substring(0, 1);
       String end = s.substring(s.length() - 2, s.length()-1);
       if(s.equals("") || s.length() == 1){
           return true;
        }
       else if(start.equalsIgnoreCase(end)){
           return palindromeTester(s.substring(1, s.length() - 1));
        }
       else{
           return false;
        }
    }

Upvotes: 1

Views: 744

Answers (3)

daniel451
daniel451

Reputation: 10992

Nice that you found out about your mistakes yourself (:

However, your solution is a bit...long...don't you think so?

For example your line:

if(s.equals("") || s.length() == 1)

could be rewritten as:

if( s.length() < 2 )

Why don't you short it a bit? For example (if you prefer recursion to do the magic) these lines solve the whole problem:

public boolean sampleMethod(String str)
{
    if(str.length() < 2)
    {
        return true;
    }
    return str.charAt(0) == str.charAt(str.length()-1) && sampleMethod(str.substring(1,str.length()-1));
}

Upvotes: 2

Ely Eastman
Ely Eastman

Reputation: 235

Woohoo! Found it! In the first example, the method created the substrings after each recursive call, regardless if the string was empty or not, generating an indexoutofbounds exception for each empty string. By creating the substrings after checking for an empty string, the program won't create the substrings if the string is empty.

Old:

String start = s.substring(0, 1);
String end = s.substring(s.length() - 2, s.length() - 1);
if (s.equals("") || s.length() == 1) {
    return true;
}

New:

if (s.equals("") || s.length() == 1) {
    return true;
}
String start = s.substring(0, 1);
String end = s.substring(s.length() - 1);

Feels so good to figure your own stuff out.

Upvotes: 3

GameDroids
GameDroids

Reputation: 5662

I found two things in your code that make it run again:

public static boolean palindromeTester(String s) {
     s = justLetters(s); //calls a method that removes all spaces, punctuation
    String start = s.substring(0, 1);

    // here: the last character in a string is at position 
    // s.length() - 1, s.length()) 
    // and not 
    // s.length() - 2, s.length()-1
    String end = s.substring(s.length() - 1, s.length()); 

     // secondly you need to test if the string has only 2 characters left and if those two characters are the same
    if (s.isEmpty() || s.length() == 1 || (s.length()==2 && s.charAt(0)==s.charAt(1))) {
        return true;
    } else if (start.equalsIgnoreCase(end)) {
        System.out.println("returning "+s.substring(1, s.length() - 1));
        return palindromeTester(s.substring(1, s.length() - 1));
    } else {
        return false;
    }
}

I realize that the recursion anchor is different now, but your OutOfBounds was thrown when your input string had an unequal number of characters.

EDIT

almost forgot: I also changed equals("") to s.isEmpty(). Use isEmpty to check if Strings are empty.

Upvotes: 2

Related Questions