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

Print all anagrams together in a sentence in Java

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

Problem

I know a lot of memory is being used, but how do I make this more efficient in space(and time) .

public class AnagramsTogether {

    public static String getAnagramsTogether(String sentence){

        if(sentence == null || sentence.trim().equals("")) return null;

        String[] words = sentence.split(" ");
        HashMap> map =new HashMap>();

        for(String word:words)
        {
            char[] wordArray = word.toCharArray();
            Arrays.sort(wordArray);

            List tempList = map.get(new String(wordArray));
            if(tempList == null) tempList = new ArrayList(); 
            tempList.add(word);
                            map.put(new String(wordArray),tempList);
        }

        StringBuilder result = new StringBuilder();
        for(Map.Entry> entry : map.entrySet())
        {

            for(String item:entry.getValue()){
                result.append(item+ " ");
            }
        }

        return result.toString();
    }

    public static void main(String[] args) {
         System.out.println(getAnagramsTogether("cat dog act tac god"));
    }

}

Solution

Whitespace discipline

While your indentation is good, the placement of whitespace isn't very good yet. In Java, we tend to put whitespace after keywords and before braces. A beautifier with standard settings would make your code more readable already.

Use the most common type that still does the job.

For example, your variable map should be declared as Map, not HashMap.

Smarter Use of Map API

Instead of getting the list, checking whether it's null and if it isn't null, creating it, and then always putting it, you could use Map.computeIfAbsent(), like this:

map
    .computeIfAbsent(normalizedAnagram, s -> ArrayList::new)
    .add(word);


Split your logic

Your function getAnagramsTogether is doing many things. You could split it into multiple methods. For example, you could have a separate method normalizeAnagram() which takes a String and returns a new String with the characters sorted.

Bug in your call to Arrays.sort(char[] wordArray)

It should be noted that char is NOT a character. It is a UTF-16 fragment. For the first Unicode plane, i.e. code points with values that can be represented by 16 bits, char is a character. But for code points with higher values, a char contains only part of the character, and the way how you construct the normalized anagram is tearing characters apart.

It may not matter to you, as probably all relevant words in your use cases would have code points in the 16 bit plane.

Don't use + in StringBuilder.append()

You should be consistent, either use + or append().
But using + inside StringBuilder.append() doesn't make sense.
We use StringBuilder in order to have String-concatenation which is faster and uses less memory than using +. Using + in an argument to StringBuilder.append() is completely contrary to the intention of using StringBuilder in the first place.

Don't construct the same object twice.

You're calling new String(wordArray) twice for no good reason.
You could just create a single String normalizedAnagram and use that.

Consider a smarter way of String concatenation.

The following code concatenates your final Map into a String in a smarter way:

map
    .entrySet()
    .stream()
    .flatMap(entry -> entry.getValue().stream())
    .collect(Collectors.joining(" "))

Code Snippets

map
    .computeIfAbsent(normalizedAnagram, s -> ArrayList::new)
    .add(word);
map
    .entrySet()
    .stream()
    .flatMap(entry -> entry.getValue().stream())
    .collect(Collectors.joining(" "))

Context

StackExchange Code Review Q#119904, answer score: 7

Revisions (0)

No revisions yet.