0

I have to write a code which should test, if string one is an anagram of another string. But my code doesn´t work. I get everytime false as result. Can anyone explain where is my mistake ?

public boolean anagram (String s1, String s2) {
    count = 0;
    for(int i = 0; i !=-1; i++) {
        if(s1.indexOf(s2.charAt(i)) != -1) {
        count++;
        temp = Integer.toString(s1.indexOf(s2.charAt(i)));
        s1.replace(temp,"");
    } if(count+1 == s1.length())
        return a == true;
    }

    return a == false;
Bastiano Coimbra
  • 11
  • 1
  • What is `a`? If `a` is true, what is `a == true`? Moreover, what is `a == false`? – mkasberg Dec 12 '18 at 17:25
  • @mkasberg Good point. – Yin Cognyto Dec 12 '18 at 17:27
  • a is a boolean: boolean a; – Bastiano Coimbra Dec 12 '18 at 17:29
  • 1
    Possible duplicate of [finding if two words are anagrams of each other](https://stackoverflow.com/questions/4236906/finding-if-two-words-are-anagrams-of-each-other) – Soutzikevich Dec 12 '18 at 18:31
  • 3
    @P.Soutzikevich And a much better answer there, compared to the attempted solution here. This method could be inconsistent in case of multiple occurences of a letter, IMHO. – Yin Cognyto Dec 12 '18 at 18:52
  • Possible duplicate of [finding if two words are anagrams of each other](https://stackoverflow.com/questions/4236906/finding-if-two-words-are-anagrams-of-each-other) – Herb Dec 12 '18 at 20:15
  • 1
    @HerbWolfe Possible duplicate of [my comment just above yours](https://stackoverflow.com/questions/53747962/i-don%c2%b4t-find-the-mistake-of-my-code-to-check-strings-for-anagram?noredirect=1#comment94351734_53747962) :P – Soutzikevich Dec 13 '18 at 17:15
  • @P.Soutzikevich Hehe, nice one :D – Yin Cognyto Dec 14 '18 at 21:09

2 Answers2

1

Try something like this:

public boolean anagram (String s1, String s2) {
    int count = 0;
    boolean a = false;
    for (int i = 0; i < s2.length(); i++) {
        int s1index = s1.indexOf(s2.charAt(i));
        if (s1index != -1) {
        count++;
        s1 = s1.replaceFirst(String.valueOf(s1.charAt(s1index)),"");
        } 
    }
    if ((count == s2.length()) && (s1.length() == 0))
        a = true;
    return a;
    }

Basically the test should be done at the end of the function and outside the for loop, and you'd want to return the boolean value of a, not the boolean value of a == true (which should be always false, since a is never assigned the true value in your code).

EDIT: I've corrected two mistakes in my initial answer, as well as added code that will hopefully eliminate the issue of multiple occurences of a letter (which is why my first answer didn't work in some cases). This code modifies the s1 string - if you don't want that, just do s1temp = s1; at the beginning of the function and replace s1 with s1temp for the rest of the function. The code also uses string's replaceFirst() method - since the replace method deletes all occurences of a letter as far as I know, but you can switch to other alternatives that delete a character from a string (see this question for further details). By the way, your temp approach was incorrectly trying to replace the index of a character, not the actual character, not to mention the fact that a i !=-1 test makes no sense in a for loop that is incrementing i starting from 0.

NOTE: You can verify if it's working (with every step printed out on the screen) here (online Java compiler).

Yin Cognyto
  • 986
  • 1
  • 10
  • 22
  • Thank you for the expression :). The solution worked :) – Bastiano Coimbra Dec 12 '18 at 17:33
  • Please consider selecting this answer as the solution to your question and also voting-up for the user's effort in solving your problem :) *Have a wonderful day!* – Soutzikevich Dec 12 '18 at 18:33
  • @BastianoCoimbra You're welcome - good luck at school ;) – Yin Cognyto Dec 12 '18 at 18:34
  • If I test for the strings "anna" and "aaaa" it also return true with this code. But why ? I can´t understand this. Because it should stop count++, after he deleted the 2 a´s of anna and should stop when he try to find the next char, because it should return -1. – Bastiano Coimbra Dec 13 '18 at 13:07
  • @BastianoCoimbra I've updated my answer to try to correct the issue you mentioned - let me know if it works. Sorry for the delay, I've been busy all day :) – Yin Cognyto Dec 14 '18 at 20:56
  • thank you, the new solution works fine. Thank you for your help :) – Bastiano Coimbra Dec 15 '18 at 16:04
  • It was actually my first Java answer (and code), LOL. I was mistakenly under the impression that it was Javascript at first, and then after answering and realizing my error I just went with the flow. You could mark an answer (not necessarily mine) as the accepted one, if it helped you, of course. – Yin Cognyto Dec 16 '18 at 21:53
1

My solution does not modify immutable Strings (i.e. create new modified string instnce) and temporary array with size 26:

public static boolean isAnagram(String one, String two) {
    one = one != null ? one.toLowerCase() : "";
    two = two != null ? two.toLowerCase() : "";

    if (one.length() != two.length())
        return false;

    int[] arr = new int[26];

    for (int i = 0; i < one.length(); i++)
        arr[one.charAt(i) - 'a']++;
    for (int i = 0; i < two.length(); i++)
        arr[two.charAt(i) - 'a']--;
    for (int ch : arr)
        if (ch != 0)
            return false;

    return true;
}
Oleg Cherednik
  • 17,377
  • 4
  • 21
  • 35