HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Detecting if two strings are anagrams using TreeMap

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
anagramsaretreemaptwousingstringsdetecting

Problem

I decided to use Java TreeMap because it seems like a great data structure to fit this problem. However, I am not sure if there could be much simpler methods using better data structure or how I could start improving the time complexity of this algorithm. I would also like to know how I can improve the style of the current code.

Is the time complexity of following algorithm \$O(n \log n)\$?

/**
 * Created by mona on 5/25/16.
 */
import java.util.Map;
import java.util.TreeMap;

public class isAnagram {

    public static boolean areAnagrams( String s1 , String s2 ) {

        if ( s1.length() != s2.length() ) {
            return false;
        }

        Map charFrequencyS1 = new TreeMap<>();
        Map charFrequencyS2 = new TreeMap<>();

        for ( int i=0 ; i<s1.length() ; i++ ) {
            if ( charFrequencyS1.containsKey(s1.charAt(i)) ) {
                int freq = charFrequencyS1.get(s1.charAt(i));
                charFrequencyS1.put(s1.charAt(i), freq+1);
            }

            else {
                charFrequencyS1.put(s1.charAt(i), 1);
            }

            if ( charFrequencyS2.containsKey(s2.charAt(i)) ) {
                int freq = charFrequencyS2.get(s2.charAt(i));
                charFrequencyS2.put(s2.charAt(i), freq+1);
            }

            else {
                charFrequencyS2.put(s2.charAt(i), 1);
            }

        }

        return charFrequencyS1.equals(charFrequencyS2);

    }

    public static void main( String args[] ) {

        String s1="mona";
        String s2="noea";

        System.out.println(areAnagrams(s1,s2));

    }
}

Solution

Replace the TreeMap with a HashMap since a TreeMap offers \$O(log(n))\$ lookup (containsKey and get) and insertion (put) time whereas a HashMap offers amortized constant or \$O(1)\$ time for those same methods.

The time complexity of this algorithm (with a HashMap) is in fact O(length(s1)) or rather \$O(n)\$ because the contains, add and remove methods of a HashMap work in amortized constant or \$O(1)\$ time (it is \$O(n)\$ when the internal arrays have to be expanded).

You can make the code cleaner by getting rid of all those ifs in the loop. The get method returns null if the key doesn't exist, which helps us remove that containsKey call.

for ( int i=0 ; i<s1.length() ; i++ ) {
        Integer freq = charFrequencyS1.get(s1.charAt(i));
        charFrequencyS1.put(s1.charAt(i), freq == null ? 1 : freq.intValue() + 1);

        Integer freq = charFrequencyS2.get(s2.charAt(i));
        charFrequencyS2.put(s2.charAt(i), freq == null ? 1 : freq.intValue() + 1);
    }


You could even go a step further and extract those two lines into a method, like

void addCharToMap(Map charMap, Character char) {
    Integer freq = charMap.get(char);
    charMap.put(char, freq == null ? 1 : freq.intValue() + 1);
}


And use the above method twice.

One thing to note is that if you are sure that your input is going to be numeric or alphanumeric, you can set the initialCapacity of the HashMap to be 26 or 36 respectively, because that amount is the maximum number of possible unique characters in the strings.

Code Snippets

for ( int i=0 ; i<s1.length() ; i++ ) {
        Integer freq = charFrequencyS1.get(s1.charAt(i));
        charFrequencyS1.put(s1.charAt(i), freq == null ? 1 : freq.intValue() + 1);

        Integer freq = charFrequencyS2.get(s2.charAt(i));
        charFrequencyS2.put(s2.charAt(i), freq == null ? 1 : freq.intValue() + 1);
    }
void addCharToMap(Map<Character, Integer> charMap, Character char) {
    Integer freq = charMap.get(char);
    charMap.put(char, freq == null ? 1 : freq.intValue() + 1);
}

Context

StackExchange Code Review Q#129336, answer score: 7

Revisions (0)

No revisions yet.