trjhtr

A couple of weeks I had a coding interview and the problem to solve was very simple: reverse the words order of a string. What did I do wrong here?

public static String reverse(String phrase) 
 String words = phrase.split(" ");
 StringBuffer sb = new StringBuffer();
 for (String word : words) 
 String reversed = new String();
 for (int j = word.length() - 1; j >= 0; j--) 
 reversed += word.charAt(j);
 
 sb = sb.append(reversed).append(" ");
 

 return sb.toString().substring(0, sb.toString().length() - 1);
 

The main reason why you fail may be because your code reverse the entire sentence while only the order of the words must be reversed; Hello the world should be world the hello. If you want to reverse the whole string, there is StringBuilder.html#reverse()

That put aside. Some problems I see is the usage of StringBuffer which is slower but synchronized. There is also this new String() in your loop which is useless because you can already use sb.append. And there is this ugly string building at the end where sb.toString() is abused while sb.delete can do it.

For the fun I made mine which revert the words by traversing the char sequence from the end and insert the character at a given index. The index is changed when a space is found to insert at the end of the new string : https://github.com/gervaisb/stackexchange-codereview/blob/q184229/src/main/java/q184229/Sentence.java

final StringBuilder target = new StringBuilder(value.length());
for (int i=value.length()-1, at=0; i>-1; i--) 
 char character = value.charAt(i);
 if ( Character.isWhitespace(character) ) 
 target.append(character);
 at = target.length();
 else 
 target.insert(at, character);
 

Thanks for your fast response guys, after reading your messages I tried to solve it different:

public static void reverse(String phrase, char delimiter) 
 char newPhrase = new char[phrase.length()];
 int start = 0, i = 0, currentPos = newPhrase.length;
 while (i < phrase.length()) i == phrase.length() - 1) 
 char word = phrase.substring(start, i).toCharArray();
 currentPos = currentPos - word.length;
 System.arraycopy(word, 0, newPhrase, currentPos, word.length);
 if (i != phrase.length() - 1) 
 newPhrase[--currentPos] = delimiter;
 
 start = ++i;
 else 
 i++;
 
 

 System.out.println(new String(newPhrase));
 

Your current solution looks to be too complex. If all you need to do is swap the order of words, Strings that are separated by white space, then why not simply do this:

• Use String#split("\s+") to split the input text into an array of words -- of Strings split greedily by white-space


• Iterate through this array backwards in a simple for loop


• Add these Strings to a StringBuilder (not a StringBuffer which has unnecessary overhead of thread-safety -- we're doing this in only one thread)


• Avoiding use of new String(...), it's almost never necessary to use this, and there is a down-side to its use, including avoiding appropriate and efficient use of the String-pool.


• And then return.

e.g., something as basic as:

public static String reverseWords(String inText) 
 StringBuilder sb = new StringBuilder();
 String tokens = inText.split("\s+");
 for (int i = tokens.length - 1; i >= 0; i--) 
 sb.append(tokens);
 if (i != 0) 
 sb.append(" ");
 
 
 return sb.toString();