Skip to main content
added 1 character in body
Source Link
janos
  • 113.1k
  • 15
  • 154
  • 396

##Technical

   groupAnagrams( args );

That should be:

    groupAnagrams( args, hm );

Bug in copy/paste somehow?

1-liner statements, even simple ones like this:

      if( list[ x ] == null ) continue;

should be braced like:

       if( list[ x ] == null ) {
           continue;
       }

This prevents maintenance problems later, and makes revision history easier to follow.

##Algorithm

Your algorithm is taking each member of the array, and comparing to each subsequent member, removing anagram matches.

The primeHash() method is interesting, but ultimately it is a red herring of sorts... and it only works for lower-case input words. You have obviously invested some thought in to it, but there's a simpler solution to that problem:

private static final String anagramKey(String word) {
    word = word.toLowerCase();
    char[] chars = word.toCharArray();
    ArrayArrays.sort(chars);
    return new String(chars);
}

Take all letters, sort them, return a String. All anagrams of the same letters will have the same keys.

With that key system, the basic code can be come simpler with:

HashMap<String,List<String>> matchMap = new HashMap<>();
for (String word : args) {
    String key = anagramKey(word);
    if (!matchMap.containsKey(key)) {
        matchMap.put(key, new ArrayList<String>());
    }
    matchMap.get(key).add(word);
}

System.out.println(matchMap);

That reduces your problem to an \$O(n)\$ one.

##Technical

   groupAnagrams( args );

That should be:

    groupAnagrams( args, hm );

Bug in copy/paste somehow?

1-liner statements, even simple ones like this:

      if( list[ x ] == null ) continue;

should be braced like:

       if( list[ x ] == null ) {
           continue;
       }

This prevents maintenance problems later, and makes revision history easier to follow.

##Algorithm

Your algorithm is taking each member of the array, and comparing to each subsequent member, removing anagram matches.

The primeHash() method is interesting, but ultimately it is a red herring of sorts... and it only works for lower-case input words. You have obviously invested some thought in to it, but there's a simpler solution to that problem:

private static final String anagramKey(String word) {
    word = word.toLowerCase();
    char[] chars = word.toCharArray();
    Array.sort(chars);
    return new String(chars);
}

Take all letters, sort them, return a String. All anagrams of the same letters will have the same keys.

With that key system, the basic code can be come simpler with:

HashMap<String,List<String>> matchMap = new HashMap<>();
for (String word : args) {
    String key = anagramKey(word);
    if (!matchMap.containsKey(key)) {
        matchMap.put(key, new ArrayList<String>());
    }
    matchMap.get(key).add(word);
}

System.out.println(matchMap);

That reduces your problem to an \$O(n)\$ one.

##Technical

   groupAnagrams( args );

That should be:

    groupAnagrams( args, hm );

Bug in copy/paste somehow?

1-liner statements, even simple ones like this:

      if( list[ x ] == null ) continue;

should be braced like:

       if( list[ x ] == null ) {
           continue;
       }

This prevents maintenance problems later, and makes revision history easier to follow.

##Algorithm

Your algorithm is taking each member of the array, and comparing to each subsequent member, removing anagram matches.

The primeHash() method is interesting, but ultimately it is a red herring of sorts... and it only works for lower-case input words. You have obviously invested some thought in to it, but there's a simpler solution to that problem:

private static final String anagramKey(String word) {
    word = word.toLowerCase();
    char[] chars = word.toCharArray();
    Arrays.sort(chars);
    return new String(chars);
}

Take all letters, sort them, return a String. All anagrams of the same letters will have the same keys.

With that key system, the basic code can be come simpler with:

HashMap<String,List<String>> matchMap = new HashMap<>();
for (String word : args) {
    String key = anagramKey(word);
    if (!matchMap.containsKey(key)) {
        matchMap.put(key, new ArrayList<String>());
    }
    matchMap.get(key).add(word);
}

System.out.println(matchMap);

That reduces your problem to an \$O(n)\$ one.

Source Link
rolfl
  • 98.1k
  • 17
  • 220
  • 419

##Technical

   groupAnagrams( args );

That should be:

    groupAnagrams( args, hm );

Bug in copy/paste somehow?

1-liner statements, even simple ones like this:

      if( list[ x ] == null ) continue;

should be braced like:

       if( list[ x ] == null ) {
           continue;
       }

This prevents maintenance problems later, and makes revision history easier to follow.

##Algorithm

Your algorithm is taking each member of the array, and comparing to each subsequent member, removing anagram matches.

The primeHash() method is interesting, but ultimately it is a red herring of sorts... and it only works for lower-case input words. You have obviously invested some thought in to it, but there's a simpler solution to that problem:

private static final String anagramKey(String word) {
    word = word.toLowerCase();
    char[] chars = word.toCharArray();
    Array.sort(chars);
    return new String(chars);
}

Take all letters, sort them, return a String. All anagrams of the same letters will have the same keys.

With that key system, the basic code can be come simpler with:

HashMap<String,List<String>> matchMap = new HashMap<>();
for (String word : args) {
    String key = anagramKey(word);
    if (!matchMap.containsKey(key)) {
        matchMap.put(key, new ArrayList<String>());
    }
    matchMap.get(key).add(word);
}

System.out.println(matchMap);

That reduces your problem to an \$O(n)\$ one.