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

Grouping Anagrams

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

Problem

Please be über brutal, and treat this as a technical interview question.

I was wondering how this function that I wrote looks like. It groups together words that have the same alphabets (i.e anagrams).

Time Complexity: \$O(n^2)\$

Space Complexity: \$O(n)\$

private static List> groupAnagrams(ArrayList words){
          List> groupedAnagrams = new ArrayList<>();
          AbstractMap sortedWords = new HashMap<>();
          Set> sameAnagramsSet = new HashSet<>();
          for(String word : words){
              char[] wordToSort = word.toCharArray();
              Arrays.sort(wordToSort);
              sortedWords.put(word, new String(wordToSort));
          }
          for(Map.Entry entry : sortedWords.entrySet() ){
              Set sameAnagrams = new HashSet<>();
              sameAnagrams.add(entry.getKey());
              for(Map.Entry toCompare : sortedWords.entrySet()){
                  if(entry.getValue().equals(toCompare.getValue())){
                      sameAnagrams.add(toCompare.getKey());
                  }
              }
              if(sameAnagrams.size() > 0){
                  sameAnagramsSet.add(sameAnagrams);
              }
          }
          for(Set set : sameAnagramsSet){
              groupedAnagrams.add(new ArrayList<>(set));
          }
          return groupedAnagrams;
      }

Solution

Use the right abstractions in method parameters and local variables, for example instead of:

private static List> groupAnagrams(ArrayList words){
      AbstractMap sortedWords = new HashMap<>();


would have been better as:

private static List> groupAnagrams(List words){
      Map sortedWords = new HashMap<>();


that is:

  • ArrayList method parameter type changed to List. Or actually, if the ordering of words is not important, then Collection would be even better.



  • AbstractMap local variable type changed to simply Map.



Instead of:

if(sameAnagrams.size() > 0){


this would be better:

if (!sameAnagrams.isEmpty()) {


If I'm reading it right, it seems the algorithm could have been simplified to a single for loop:

private List> groupAnagrams(List words) {
    Map> groupedAnagramsMap = new HashMap>();
    for (String word : words) {
        char[] wordToSort = word.toCharArray();
        Arrays.sort(wordToSort);
        String key = new String(wordToSort);
        List anagrams = groupedAnagramsMap.get(key);
        if (anagrams == null) {
            anagrams = new ArrayList();
            groupedAnagramsMap.put(key, anagrams);
        }
        anagrams.add(word);
    }
    return new ArrayList>(groupedAnagramsMap.values());
}


This implementation produces equivalent list of list, if you don't mind the ordering of items.

If you could forgive changing the method signature to return Collection instead of list, then the last new ArrayList could be omitted, for cleaner and better result:

private Collection> groupAnagrams(List words) {
    // ... same as above
    return groupedAnagramsMap.values();
}

Code Snippets

private static List<List<String>> groupAnagrams(ArrayList<String> words){
      AbstractMap<String, String> sortedWords = new HashMap<>();
private static List<List<String>> groupAnagrams(List<String> words){
      Map<String, String> sortedWords = new HashMap<>();
if(sameAnagrams.size() > 0){
if (!sameAnagrams.isEmpty()) {
private List<List<String>> groupAnagrams(List<String> words) {
    Map<String, List<String>> groupedAnagramsMap = new HashMap<String, List<String>>();
    for (String word : words) {
        char[] wordToSort = word.toCharArray();
        Arrays.sort(wordToSort);
        String key = new String(wordToSort);
        List<String> anagrams = groupedAnagramsMap.get(key);
        if (anagrams == null) {
            anagrams = new ArrayList<String>();
            groupedAnagramsMap.put(key, anagrams);
        }
        anagrams.add(word);
    }
    return new ArrayList<List<String>>(groupedAnagramsMap.values());
}

Context

StackExchange Code Review Q#47739, answer score: 6

Revisions (0)

No revisions yet.