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

Reverse a string word by word

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

Problem

Given an input string, reverse the string word by word.

For example: Given s = "the sky is blue", return "blue is sky the".

What constitutes a word?

  • A sequence of non-space characters constitutes a word.



Could the input string contain leading or trailing spaces?

  • Yes. However, your reversed string should not contain leading or trailingspaces.



How about multiple spaces between two words?

  • Reduce them to a single space in the reversed string.



public static String reverseWords(String s){
    StringTokenizer strTok = new StringTokenizer(s, " ");
    Stack stack=new Stack();
    StringBuilder buff=new StringBuilder();
    while(strTok.hasMoreElements()) {
        String str = (String)strTok.nextToken();
        if(!str.equals("")) stack.push(str);
    }
    while(!stack.isEmpty()){
        buff.append(stack.pop()); 
        if(!stack.isEmpty()) buff.append(" ");
    }
    return buff.toString();
 }

Solution

-
Stack seems more or less deprecated. Javadoc says the following:


A more complete and consistent set of LIFO stack
operations is provided by the Deque interface and its
implementations, which should be used in preference to this class.

I guess an ArrayDeque is a good choice here.

-
It's seems that StringTokenizer is also deprecated. Javadoc says the following:


StringTokenizer is a legacy class that is retained for
compatibility reasons although its use is discouraged in new code.
It is recommended that anyone seeking this functionality use the
split method of String or the java.util.regex package instead.

-
I'd rename

  • buff to result



  • stack to words



  • str to word



to describe their purpose. It would be easier to read and help understanding code.

-
You could move the declaration of buff right before the second while loop.

StringBuilder buff = new StringBuilder();
while (!stack.isEmpty()) {
    buff.append(stack.pop());
    if (!stack.isEmpty())
        buff.append(" ");
}


Try to minimize the scope of local variables. It's not necessary to declare them at the beginning of the method, declare them where they are first used. (Effective Java, Second Edition, Item 45: Minimize the scope of local variables)

-

if(!str.equals(""))


You could use String.isEmpty() here. It's easier to read since it says in a similar language to English what does this condition do.

-

if(!str.equals("")) stack.push(str);


According to the Code Conventions for the Java Programming Language,


if statements always use braces {}.

Omitting them is error-prone. I've found the one-liner above hard to read because if you scan the code line by line it's easy to miss that at the end of the line there is a statement (push).

-
It's good to know that there is a similar function in Apache Commons Lang: StringUtils.reverseDelimited. You can check the implementation online for other ideas.

(See also: Effective Java, 2nd edition, Item 47: Know and use the libraries The author mentions only the JDK's built-in libraries but I think the reasoning could be true for other libraries too.)

Here is the modified code with the Deque and other suggestions:

public static String reverseWords(String input) {
    Deque words = new ArrayDeque<>();
    for (String word: input.split(" ")) {
        if (!word.isEmpty()) {
            words.addFirst(word);
        }
    }
    StringBuilder result = new StringBuilder();
    while (!words.isEmpty()) {
        result.append(words.removeFirst());
        if (!words.isEmpty()) {
            result.append(" ");
        }
    }
    return result.toString();
}

Code Snippets

StringBuilder buff = new StringBuilder();
while (!stack.isEmpty()) {
    buff.append(stack.pop());
    if (!stack.isEmpty())
        buff.append(" ");
}
if(!str.equals(""))
if(!str.equals("")) stack.push(str);
public static String reverseWords(String input) {
    Deque<String> words = new ArrayDeque<>();
    for (String word: input.split(" ")) {
        if (!word.isEmpty()) {
            words.addFirst(word);
        }
    }
    StringBuilder result = new StringBuilder();
    while (!words.isEmpty()) {
        result.append(words.removeFirst());
        if (!words.isEmpty()) {
            result.append(" ");
        }
    }
    return result.toString();
}

Context

StackExchange Code Review Q#43838, answer score: 18

Revisions (0)

No revisions yet.