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.

For CS class, I had to reverse the digits of an integer from the input. This is my solution:

System.out.println(new StringBuilder(new Integer(new Scanner(System.in).nextInt()).toString()).reverse().toString());

My main question is this: is this too golfed?

Obviously I could simply add whitespace and comments, but is this level of directness practical?

Also, is there a better way to do this? Perhaps something like the following:

final String string = new Integer(new Scanner(System.in).nextInt()).toString();
for (int i = string.length(); i != -1; System.out.print((i-- != 0) ? string.charAt(i) : "\n"));
share|improve this question
9  
Well if you want all your program in one line this is excellent. If you want maintainable and readable code this is really not a good way to go! –  Marc-Andre Mar 3 at 0:13
2  
Hi, I have downvoted your question. Please see our help center to see what we're all about. You could edit the question to include working code that you want peer reviewed; a golfed one-liner isn't going to get you much of a useful code review ;) –  Mat's Mug Mar 3 at 0:16
2  
heh, upvoting, because, technically, this is asking to ungolf some code.... –  rolfl Mar 3 at 0:19
    
@Mat'sMug I didn't really mean to golf it; I just don't like to have (let me count) up to four middlemen variables. I am sort of asking to ungolf it. –  Simon Kuang Mar 3 at 0:27
    
@Marc-Andre Yeah. Can you help with that in an answer? –  Simon Kuang Mar 3 at 0:29
show 2 more comments

5 Answers 5

up vote 16 down vote accepted

In Java, and in Code Review, the preference is for creating both highly readable, and highly efficient code.

It is an almost bizarre fact, that one leads to the other.

your code has one efficiency, and that is space-on-disk.... it is neither readable, nor high-performance.

System.out.println(new StringBuilder(new Integer(new Scanner(System.in).nextInt()).toString()).reverse().toString());
  • For a start, having to scroll off the page means you have to read with your mouse, not with your eyes.

  • Secondly, it is very hard to evaluate what parenthesis match when they are all clumped together.

Breaking your code down (which should not be necessary), we have:

int valinput = new Scanner(System.in).nextInt();
String stringinput = new Integer(valinput).toString();
String reversed = new StringBuilder(stringinput).reverse().toString();
System.out.println(reversed);

Now, you need error handling on the Scanner, and it needs to be closed as well. Without that, you have a program that is ugly when a user types in 123.0.

Since you have no error handling, you may as well just junk the lines with the Integer...

And, there is no reason to convert the StringBuilder to a String directly...

Without error handling, you can:

System.out.println(new StringBuilder().append(scanner.nextInt()).reverse());

That is, effectively equivalent to your code, but it has different error-condition characteristics..

Also, it is, suprisingly, quite readable, and fast.

Now, all you need is the try/catch for the scanner, and you are sorted.

share|improve this answer
    
Along the lines of being clear and concise, I think String.valueOf(valInput) is much nicer than new Integer(valinput).toString(). –  DaoWen Mar 3 at 0:41
1  
When I do System.out.println(new StringBuilder(new Scanner(System.in).nextInt()).reverse());, I get an empty line. –  Simon Kuang Mar 3 at 4:03
    
@SimonKuang you are right, and it's a stupid mistake... fixing now. –  rolfl Mar 3 at 4:24
add comment

@Mat'sMug I didn't really mean to golf it; I just don't like to have (let me count) up to four middlemen variables. I am sort of asking to ungolf it.

Well this is the root of all one-liner code. You don't want middlemen variables, but why ? Breaking the code with middlemen variables is the first step to have readable code. There is a tons of reasons to have middlemen variables. Debugging will be easier, changing implementation of functionality will be easier, re-factoring the code will be easier and your code will be easier to test (That is very very important).

Let's see what you have :

  System.out.println(new StringBuilder(new Integer(new Scanner(System.in).nextInt()).toString()).reverse().toString());

Let's start by breaking this :

Scanner input = new Scanner(System.in);
int integerToReverse = input.nextInt();
StringBuilder reverser = new StringBuilder();
reverser.append(integerToReverse);
reverser.reverse();
String reversed = reverser.toString();
System.out.println(reversed);

By breaking this in smaller steps, we can observe what have goes wrong. I've added middlemen variables, but you can observe but I don't need an Integer.

By breaking the code, you could see that you can extract the code that reverse the int. If you need to reverse multiple times, you could just call the method with the int to reverse and you won't need duplicated code. This will help to encapsulate everything in his own "box" and you could changed each box more easily.

What if the user doesn't input an int ? There is a chance that the user doesn't input an integer. Your code isn't ready for that. You should read the documentation for Scanner.nextInt() and come up with something that will be more solid.

Note :

I though that you had a bug in your code. I was reading as you were building a StringBuilder with an Integer. This would be a bug, but it turns out that I was wrong. This is one of the consequence of one liner. You can't read it right at first. Everything is so compact that you need to de-compact it in your head or in code to understand it. This lead to bug by people that won't understand your code, change it because they need to add something (functionality, logging,etc) and will break it because they didn't break it at the right place.

share|improve this answer
    
This might be too much for a comment, but won't middlemen variables get messy and use more memory/slow GC? –  Simon Kuang Mar 3 at 8:03
3  
@SimonKuang your concern is valid. Apparently they do require collection by the GC: stackoverflow.com/questions/19268406/… However, as mentioned there, premature optimization is undesirable. We should focus on readability first, and optimize only when a bottleneck is detected. –  joeytwiddle Mar 3 at 9:32
1  
In a small program like this they very very small overhead is negligible. And as joeytwiddle said, readability is a much more important factor in most cases. –  Max Mar 3 at 11:34
    
With code this simple, it probably is better to duplicate. You may need to call it with different ints, but it's equally likely that you may need additional string processing in some of those instances, so why not leave the caller responsible for creating its own StringBuilder? You're essentially only encapsulating append and reverse. –  nmclean Mar 3 at 13:09
2  
@nmclean For a CS class, I'm no so sure that duplicating is the way to go. Yes you're essentially encapsulating append and reverse but the major is that you're also encapsulating a functionality. You're organizing your code. –  Marc-Andre Mar 3 at 14:15
add comment

Why not throw in some arithmetics?

private static String reverseInt(int i) {
    StringBuilder sb = new StringBuilder();
    for(int d = i; d != 0; d /=10) {
        sb.append(d % 10);
    }
    return sb.toString();
}
share|improve this answer
add comment

While using a String operation on this is probably the easiest and the fastest way to reverse an interger, you could also do it without casting

NOTE: this only works with integers, not with any decimal numbers.

int number = 12345;
int result = reverseInteger(number);

public int reverseInteger(int number) {
    int reverse = 0;

    while(number > 0) {
        reverse *= 10;
        reverse += number % 10;
        number /= 10;
    }

    return reverse;
}

// To get a String as result, use String.valueOf(reverse);

/*
-- start --
reverse = 0
number = 12345
-- first run --
reverse = 0 * 10 + 5 = 5
number = 1234
-- second run --
reverse = 5 * 10 + 4 = 54
number = 123
-- third run --
reverse = 54 * 10 + 3 = 543
number = 12
-- fourth run --
reverse = 543 * 10 + 2 = 5432
number = 1
-- fifth run --
reverse = 5432 * 10 + 1 = 54321
number = 0
*/
share|improve this answer
1  
That way you lose zeros at the end, e.g. reverseInteger(12300) gives you 321. –  Landei Mar 8 at 11:36
    
Darn you're right ... I forgot how I used to fix that. You could do it by checking the length and adding zeroes, but that'll take you back to String-operations or it'd add a lot more code. –  GroundZero Mar 9 at 19:40
    
For reversing an Integer, this doesn't matter. If you would want to keep the preceding zeroes, for some String operation, you won't be able to use this code –  GroundZero Mar 9 at 19:42
add comment

You could also go the character array route, which is the fastest in runtime. Here is a version that works with negative numbers:

public String reverseString(String str) {
    if (str == null) return null;
    char[] chars = str.toCharArray();
    int rightmost = (chars[0] == '-') ? 0 : 1;
    for (int i = (rightmost + 1) % 2; i < chars.length / 2; i++) {
        int tmp = chars[i];
        chars[i] = chars[chars.length - rightmost - i];
        chars[chars.length - rightmost - i] = tmp;
    }
    return new String(chars);
}
share|improve this answer
    
Does this work with negative numbers? –  GroundZero Mar 4 at 19:16
    
So -1234 would become 4321-. What happens if you parse this to a number? –  GroundZero Mar 4 at 19:18
add comment

protected by rolfl Mar 4 at 0:34

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

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