current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote -4 down vote favorite

I wrote this program which reverses the words in a given string. Please review and give your inputs.

private static void reverseWords(String value) {
    // reverse the whole string
    value = reverseInOofN2Time(value);

    // now split (on space) the string into an string array
    String[] strArr = value.split(" ");

    // make it empty
    value = "";
    for (int i = 0; i < strArr.length; i++) {
        // reverse each strArr element and append it to the value
        value += reverseInOofN2Time(strArr[i]) + " ";
    }

    System.out.println(value);
}

public static String reverseInOofN2Time(String h) {
    char[] charArr = h.toCharArray();
    for (int i = 0; i < charArr.length / 2; i++) {
        char temp = charArr[i];
        int targetPos = (charArr.length - 1) - i;
        charArr[i] = charArr[targetPos];
        charArr[targetPos] = temp; 

    }
    return new String(charArr);
}
share|improve this question

closed as off-topic by Snowhawk04, Hosch250, glampert, Quill, RobH Jul 2 at 8:02

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.
1  
Why is it called reverseInOofN2Time when the time is \$O(n)\$ and not \$O(n^2)\$? And in fact, why do you even reverse the string only to reverse it back later? –  JS1 Jul 2 at 2:07
2  
Where is the swap method? Where is this c variable? This looks like a snippet and/or broken. –  Legato Jul 2 at 3:24
    
I am sorry I missed to add the swap method, actually the intention was to get my logic in "reverseWords" method gets reviewed. I had renamed the variables by editing my post in stackoverflow, and I missed to replace "c" with "charArr". –  here4learn Jul 2 at 13:44
    
I don't see reverseInOn2Time() anywhere in the code, either. –  Hosch250 Jul 2 at 15:01
1  
Don't worry, now you know for the next time! –  Hosch250 Jul 2 at 15:12

2 Answers 2

up vote 6 down vote accepted

Your logic gets a little confusing to when you get to the for loop.

To me, when you were reversing the string at the very beginning and then splitting it up into words, I figured you were going to append the words in reverse order in to a blank string.

Then, for some reason, you start reversing all the words in strArr.

It makes more sense with the beginning of your logic to continue as I thought it was going to continue.

Here is little bit more into my idea:

"abc def"

First, reverse the string:

"fed cba"

Split the string up by spaces:

["fed", "cba"]

Right the elements of the array in reverse order to a blank string, putting a space between each word:

"cba def"

Here is my solution in Java:

value = reverseInOn2Time(value);
String[] words = value.split(" ");
StringBuilder blank = new StringBuilder();

for(int i = words.length - 1; i > -1; i--) {
    blank.append(words[i] + " ");
}

return blank.toString();

I used a StringBuiler to increase efficiency because StringBuilders are mutable, unlike Strings.

Another solution you could have is to first split up the string into words, then reverse them, and then stick them all into a blank.

Here is a little bit more into that option:

"abc def"

First, split up the string by space:

["abc", "def"]

Reverse each element of the array:

["cba", "def"]

Write the elements of the array to a blank string:

"cba def"

Here is that solution in Java:

String[] words = value.split(" ")
StringBuilder blank = new StringBuilder();

for(int i = 0; i < words.length; i++) {
    words[i] = reverseInOn2Time(words[i]);
}

for(i = 0; i < words.length; i++) {
    blank.append(words[i] + " ");
}

return blank.toString();

I personally would choose the first solution.

These snippets assume that reverseInOn2Time reverses a string, as stated by the comments near the function call.

share|improve this answer
up vote 5 down vote

Strings are immutable, for the value you later "empty", you should be using a StringBuilder for its mutability, which I doubly recommend since it comes packed with a reverse method.

Reversing a string

public static String reverse(String str) {  
        return new StringBuilder(str).reverse().toString();
    }

So if you wanted to individually reverse the words, simply split on the spaces, reverse using StringBuilder, and append to a second result StringBuilder.

Reversing words, individually.

public static String reverseWords(String str) {
        StringBuilder result = new StringBuilder();

        for (String s : str.split("\\s+")) {
            result.append(' ').append(reverse(s));
        }

        return result.substring(1);
    }
share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.