String index out of range: -1 error with loops

Question

I checked old topics with this problem and couldn't fix it.

This method is meant to compare two strings, and if every letter/character of the first string is found in the second string (not necessarily vice versa), then the method should return "true" (even if the second string has extra letters).

My idea is to check the letter at every index of the first string, see if it's in the second string, and if it is, delete that letter in both strings. When the first string runs out of letters (length equals zero) then the boolean should return true.

I think my loops or substring reaches out of range at some point.

public boolean isFound(String first, String second) {
    StringBuilder sb1 = new StringBuilder(first);
    StringBuilder sb2 = new StringBuilder(second);

    first.toCharArray();
    second.toCharArray();
    for (int i = 0; i < first.length(); i++) {
        int k = (first.substring(i, i + 1)).indexOf(second, i);
        if (sb1.length() > 0) {
            sb1.deleteCharAt(k);
            sb2.deleteCharAt(k);
        }
    }
    if (sb1.length() == 0) {
        return true;
    } else {
        return false;
    }
}

Ex: "at" and "tack" should return true, "tree" and "ere" should return false.

EDIT After reviewing the comments, my new code is now this. It always returns false, though even with "tree" and "tree".

   public boolean isFound(String first, String second){
   StringBuilder sb2 = new StringBuilder(second); 

   for(int i=0;i<first.length();i++){
   int k = sb2.indexOf(first,i);
   if (k==-1)
       return false;
   else sb2.deleteCharAt(k);
}
      return true;
}

If you have a letter multiple times in `first` does it have to be at least the same times in `second`? e.g. should `isSuperAnagram("aa", "a")` return `false`? — halex, Mar 27 '15 at 06:18

halex · Accepted Answer · 2015-03-27T06:38:22.187

4

You have a number of problems in your code.

You only need one StringBuilder version, that of second
The calls to toCharArray() are superfluous
You should not search for each character of first in second but in the mutable version of it sb2.
You are using indexOf wrong. This method should be called on the StringBuilder object to search for the first argument, you have it swapped.

The pseudocode that you can use is

isSuperAnagram(String first, String second) {
    sb = StringBuilder(second)
    for all chars in first {
        k = search index of current char of first in sb
        if(k == -1) // char is not in sb
            return false
        else
            remove char at index k from sb
    }
    return true
}

edited Mar 27 '15 at 06:38

answered Mar 27 '15 at 06:32

halex

16,253
5
58
67

Thank you, I made those changes and I think I did it wrong. It always returns "false" as if the letter is never found in sb2. `public boolean isSuperAnagram(String first, String second){ StringBuilder sb2 = new StringBuilder(second); for(int i=0;i – jessca Mar 27 '15 at 14:28
1

@jessca Your code is almost correct :). The problem you have is in the call to `indexOf`. You dont want the index in `sb2` where you can find the whole string `first` but where you can find the ith character of `first`. You also want to search the whole `sb2` every time and not start the search at index i if you look for the ith character of `first`. Long speech short code -> The line should be `int k = sb2.indexOf(first.substring(i,i+1));` – halex Mar 27 '15 at 15:03

score 1 · Answer 2 · answered Mar 27 '15 at 06:31

please review your algorithm and code/usage of APIs

` first.toCharArray();`

second.toCharArray();

wont convert first,second to array, this API would return a character array.

(first.substring(i,i+1)).indexOf(second,i); will search the whole substring2 in first.substring.

review the algo/code correct this accordingly.please take care of all the edge/corner cases.

ideal method will be to use a hashmap.(on cost of extra space)

score 0 · Answer 3 · edited Mar 27 '15 at 08:27

There is simple logic you can implement it your code

 for(int i=0;i<sb1.length;i++)
 {
  for(int j=0;j<sb2.length;j++)
   {
    if(sb1.[i]==sb2.[j])     
     { 
       count++;
       break;
     }
      if(count>=sb1.length)
      {
        System.out.print("match");
      }
   }

 }

if you want more condition then post it below my post i wrote for only your follow example Ex: "at" and "tack" should return true, "tree" and "ere" should return false.

String index out of range: -1 error with loops

3 Answers3