Java reverse string

Question

I'm trying to make a code that reverse's a string, I know there are more easy ways to do this but I just wanted to know why this code does not work:

public class Reverse
{
    /**
     * Prints a post backwards to hide its contents.
     * @param post the post to be reversed.
     */
    public String reverse(String post)
    {
        String newpost = "";
        for (int i = post.length(); i > 0; i++);
        {
            String letter = post.substring(post.length() - 1, post.length()); // gets the last character of the post
                post = post.substring(0,post.length()-1); //removes the last character of the post
            newpost = newpost + letter; //adds the last character of the post to newpost
    }
    return newpost;
}

Result:

Input: How did I ever program without loops?!

Actual: !

Expected: !?spool tuohtiw margorp reve I did woH

Input: That's so backwards!

Actual: !

Expected: !sdrawkcab os s'tahT

it gets the last letter of the post, than removes that from the post and than adds that to newpost, so you should get with input: hello: o than ol than oll than olle tahn olleh — , Mar 08 '15 at 20:52
You're incrementing when you should be decrementing. and in the for loop do `i >= 0` . — Brunaldo, Mar 08 '15 at 20:59
@Brunaldo: Decrementing is correct, but `i >= 0` will actually lead to a `StringIndexOutOfBoundsException`. — Marvin, Mar 08 '15 at 21:53

score 3 · Accepted Answer · edited May 23 '17 at 12:33

3

You have an extra ; in the line of your loop:

for (int i = post.length(); i > 0; i++);

So your replacement code is only executed once. This is easy to spot if you use an editor with code formatting abilities (in my case Eclipse, but any other IDE should do).

And for the sake of completeness - the probably easiest (built-in) way to reverse a string:

(new StringBuilder(post)).reverse().toString()

Update:

As McT and Pshemo already pointed out, the extra ; is not your only problem. You need to decrement i in your loop, otherwise you will run into a StringIndexOutOfBoundsException in line

String letter = post.substring(post.length() - 1, post.length());

when post has become the empty string.

And since we already started talking about good practices: You should do string operations within a loop using a StringBuilder for performance reasons (although these are probably not your main concern right now), cf. this Stack Overflow question.

edited May 23 '17 at 12:33

Community

1
1

answered Mar 08 '15 at 20:56

Marvin

13,325
3
51
57

I didn't see your StringBuilder submission before I posted. For completeness you should add parentheses around (new StringBuilder(strin)).reverse().toString(). Otherwise great job. – lacraig2 Mar 08 '15 at 21:03
@lacraig2: The parentheses are actually not needed. – Marvin Mar 08 '15 at 21:27
2

Not needed but a good practice. When you get a lot of new ClassA(new ClassB()) it gets confusing. That is the point of parenthesis. – lacraig2 Mar 08 '15 at 21:28
1

@lacraig2: Valid reason, added. – Marvin Mar 08 '15 at 21:33

score 1 · Answer 2 · answered Mar 08 '15 at 21:11

Just to add to Marvin's answer above you also should decrement i rather than increment it

for (int i = post.length(); i > 0; i--)

This along with what Marvin said will give you the correct solution, Although there are much easier ways to do this!

score 0 · Answer 3 · answered Mar 08 '15 at 21:01

The guy above corrected your code. He did a great job. However, in java you can do it easily in one line. Here is a better solution:

 String reverse = (new StringBuffer(initialString)).reverse().toString();

If you set a new StringBuffer to the string you want and then reverse and then set it to a string you can avoid loops altogether.

Here it is all together:

public class Reverse{
     public String reverse(String post){
        return ((new StringBuffer(post)).reverse().toString());
    }       
}

Hope it helps.

Java reverse string

3 Answers3