patternjavaMinor
Grouping Anagrams
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)\$
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:
would have been better as:
that is:
Instead of:
this would be better:
If I'm reading it right, it seems the algorithm could have been simplified to a single
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
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:
ArrayListmethod parameter type changed toList. Or actually, if the ordering of words is not important, thenCollectionwould be even better.
AbstractMaplocal variable type changed to simplyMap.
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.