patternjavaMinor
Print all anagrams together in a sentence in Java
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
Smarter Use of
Instead of getting the list, checking whether it's
Split your logic
Your function
Bug in your call to
It should be noted that
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
You should be consistent, either use
But using
We use
Don't construct the same object twice.
You're calling
You could just create a single
Consider a smarter way of String concatenation.
The following code concatenates your final Map into a String in a smarter way:
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 APIInstead 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.