How can i get this equation to work within this nested if/else statement for c#?-CodePudding

After i get pass 20 on the user end my if/else doesn't pick up the next variation of the equatuion. I know its because of setting the total = (numberofchecks * twentychecks) (fee); but i can't figure out what to use to get it to move to the next variation of the equation. The negative, over 1000, and format exceptions work.

total = (numberofchecks * twentychecks) (fee);

            TotalLabel.Text = total.ToString("C");

            // if else (5 or 6)

            if ( numberofchecks < 20 && numberofchecks == 0)
            {
                total = (numberofchecks * twentychecks)   (fee);
               
            }
            else if ( numberofchecks < -1)
            {
                MessageBox.Show("Number of checks must be between 0 and 1000");
            }
            else if (numberofchecks == 20 && numberofchecks <= 39)
            {
                total = (numberofchecks * thirtyninechecks)   (fee);
                
            }
            
            else if (numberofchecks == 40 && numberofchecks <= 59)
            {
                total = (numberofchecks * fiftyninechecks)   (fee);
                
            }
            else if (numberofchecks == 60 && numberofchecks <=1000)
            {
                total = (numberofchecks * thousandchecks)   (fee);
                
            }
            else if (numberofchecks > 1000)
            {
               MessageBox.Show("Number of checks must be between 0 and 1000");
            }
            
        }
        catch(FormatException )
        {
            MessageBox.Show(" Please enter a number");
        }

this is a screenshot of what happens when i test numbersof above twenty

CodePudding user response：

First, of we look at just one of your if statements we see the following

else if (numberofchecks == 20 && numberofchecks <= 39)

Well if numberofchecks equals 20, then it will always be <= 39, so the second part is redundant. And this is the same for all of your statments.

I think what you actually mean is

else if (numberofchecks >= 20 && numberofchecks <= 39)

Then, this would make it more logical.

This should solve your immediate problem. But we can improve it further.

It is generally better to first check the data input is valid. So this should be done first. You could even move this to another method. (Useful if you have lots of validation to perform). Then you can perform your calculation

if ( numberofchecks < -1 || numberofchecks > 1000)
{
        MessageBox.Show("Number of checks must be between 0 and 1000");
}
else
{    
    //Rest here
}

But if you are using C#9 or above, you can actually move your if statements to a switch statement which supports ranges. And in this case, we can use the default case for the invalid data as well.

switch (numberofchecks)
{
    case >= 0 and < 20:
        total = (numberofchecks * twentychecks)   (fee);
        break;
    case >= 20 and <= 39:
        total = (numberofchecks * thirtyninechecks)   (fee);
        break;
    case >= 40 and < 59:
        total = (numberofchecks * fiftyninechecks)   (fee);
        break;
    case >= 60 and < 1000:
        total = (numberofchecks * thousandchecks)   (fee);
        break;
    default:
        MessageBox.Show("Number of checks must be between 0 and 1000");
        break;
}

CodePudding user response：

Looking at the code, seems the conditions are not set up correctly.

A better conditional set up is to always check for inappropriate cases first.

So the following checks scope can be reduced to valid ones.

// Check inappropriate cases first to reduce the scope of other checks
if (numberofchecks < 0 || numberofchecks >= 1000) {
    MessageBox.Show("Number of checks must be between 0 and 1000");
} else {
    // Guess you don't want 20 to satisfy the twentychecks as well? 
    if (numberofchecks < 20) {
        // cases: 0 to 19
        total = (numberofchecks * twentychecks)   (fee);
    } else if (numberofchecks < 40) {
        // cases: 20 to 39
        total = (numberofchecks * thirtyninechecks)   (fee);
    } else if (numberofchecks < 60) {
        // cases: 40 to 59
        total = (numberofchecks * fiftyninechecks)   (fee);
    } else {
        // cases: 60 to 999
        total = (numberofchecks * thousandchecks)   (fee);
    }
    TotalLabel.Text = total.ToString("C");
}