Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C# increment ToString

I add an unexpected behaviour from C#/WPF

    private void ButtonUp_Click(object sender, RoutedEventArgs e)
    {
        int quant;
        if( int.TryParse(Qnt.Text, out quant))
        {
            string s = ((quant++).ToString());
            Qnt.Text = s;
        }
    }

So, if I get quant as 1, quant will be incremented to 2. But the s string will be 1. Is this a question of precedence?

EDIT:

I re-wrote this as:

            quant++;
            Qnt.Text = quant.ToString();

and now this works as I expected.

like image 438
sergiol Avatar asked Oct 21 '11 10:10

sergiol


2 Answers

You are using the post-increment operator. This evalutates to the original value, and then increments. To do what you want in a one-liner you can use the pre-increment operator instead.

(++quant).ToString();

But even better would be to avoid all such pitfalls and do it like this:

quant++;
string s = quant.ToString();

With the first version you have to think about the order in which things happen. In the second version no thought is required. Always value code clarity more highly than conciseness.

It's easy to believe that the one-line version is somehow faster, but that's not true. It might have been true back in the day in 1970s C systems, but even then that I doubt.

like image 76
David Heffernan Avatar answered Sep 24 '22 10:09

David Heffernan


The problem is that you're using a post-increment instead of a pre-increment... but why would you want to write this convoluted code? Just separate out the side-effect (incrementing) and the ToString call:

if (int.TryParse(Qnt.Text, out quant))
{
    quant++;
    Qnt.Text = quant.ToString();
}

Or even forego the actual increment given that you're not going to read the value again:

if (int.TryParse(Qnt.Text, out quant))
{
    Qnt.Text = (quant + 1).ToString();
}

Where possible, avoid using compound assignment in the middle of other expressions. It generally leads to pain.

Additionally, it feels like all this parsing and formatting is hiding the real model, which is that there should be an int property somewhere, which might be reflected in the UI. For example:

private void ButtonUp_Click(object sender, RoutedEventArgs e)
{
    // This is an int property
    Quantity++;
    // Now reflect the change in the UI. Ideally, do this through binding
    // instead.
    Qnt.Text = Quantity.ToString();
}
like image 44
Jon Skeet Avatar answered Sep 26 '22 10:09

Jon Skeet