Reducing the number of if statements

Question

I have the following program so far:

using System;
namespace ParkingTicket
{
    class Program
    {
        static void Main()
        {
            int speed;
            int yrInSchool;
            double fine;
            char choice = ' ';
            do
            {
                Console.Clear();
                speed = GetSpeed();
                if (speed <= 15)
                    Console.WriteLine("No speeding fine to pay.");
                else
                {
                    yrInSchool = GetYrInSchool();
                    fine = CalculateFine(speed, yrInSchool);
                    DisplayFine(fine);
                }
                choice = GetUserChoice();
            } while (choice != 'Q' && choice != 'q');
        }
        static int GetSpeed()
        {
            int speed;
            string userInput;
            try
            {
                Console.Write("Please enter the speed you were traveling: ");
                userInput = Console.ReadLine();
                speed = Convert.ToInt32(userInput);
            }
            catch
            {
                Console.WriteLine("\a\n INVALID - PLEASE TRY AGAIN");
                Console.Write("Please press enter to continue....");
                userInput = Console.ReadLine();
                speed = GetSpeed();  // this is the recursion - calling myself

            }
            return speed;

            // code this method
        }
        static int GetYrInSchool()
        {
            string userEntry;
            int year;

            /*************************************************************
             *  modify this method to validate the year using a Try/Catch
             *************************************************************/
            Console.WriteLine("\nClassifications");
            Console.WriteLine("\tFreshman  (enter 1)");
            Console.WriteLine("\tSophomore (enter 2)");
            Console.WriteLine("\tJunior    (enter 3)");
            Console.WriteLine("\tSenior    (enter 4)");

            try
            {
                Console.Write("Enter choice: ");
                userEntry = Console.ReadLine();
                year = Convert.ToInt32(userEntry);
            }

            catch
            {
                Console.WriteLine("\a\n INVALID - PLEASE TRY AGAIN");
                Console.Write("Please press enter to continue....");
                userEntry = Console.ReadLine();
                year = GetYrInSchool();  // this is the recursion - calling myself
            }
            return year;
        }
        static double CalculateFine(int speed, int year)
        {
            const double COST_PER_5_OVER = 87.50;
            const int SPEED_LIMIT = 15;
            const double INITIAL_FEE = 75.00;
            double fine = 0;

            if (((year == 1) && (speed >= 15) || (speed <= 19)))
            {
                fine = INITIAL_FEE - 50.00;
            }
            else if (((year == 1) && (speed >= 20) || (speed >= 24)))
            {
                fine += (INITIAL_FEE - 50.00) + COST_PER_5_OVER;
            }
            else if (((year == 1) && (speed >= 25) || (speed <= 29)))
            {
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 2);
            }
            else if (((year == 1) && (speed >= 30) || (speed <= 34)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 3);
            else if (((year == 1) && (speed >= 35) || (speed <= 39)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 4);
            else if (((year == 1) && (speed >= 40) || (speed <= 44)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 5);
            else if (((year == 1) && (speed >= 45) || (speed <= 49)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 6);
            if (((year == 1) && (speed >= 50) || (speed <= 54)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 7);
            if (((year == 1) && (speed >= 55) || (speed <= 59)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 8);
            if (((year == 1) && (speed >= 60) || (speed <= 64)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 9);
            if (((year == 1) && (speed >= 65) || (speed <= 69)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 10);
            if (((year == 1) && (speed >= 70) || (speed <= 74)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 11);
            if (((year == 1) && (speed >= 75) || (speed <= 79)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 12);
            if (((year == 1) && (speed >= 80) || (speed <= 84)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 13);
            if (((year == 1) && (speed >= 85) || (speed <= 89)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 14);
            if (((year == 1) && (speed >= 90) || (speed <= 94)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 15);
            if (((year == 1) && (speed >= 95) || (speed <= 99)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 16);
            if (((year == 1) && (speed >= 100) || (speed <= 104)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 17);
            if (((year == 1) && (speed >= 105) || (speed <= 109)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 18);
            if (((year == 1) && (speed >= 110) || (speed <= 114)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 19);
            if (((year == 1) && (speed >= 115) || (speed <= 119)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 20);
            if (((year == 1) && (speed >= 120) || (speed <= 124)))
                fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 21);
            else if (((year == 2) && (speed >= 16) || (speed <= 19)))
                fine = INITIAL_FEE;
            if (((year == 2) && (speed >= 20) || (speed <= 24)))
                fine = INITIAL_FEE + (COST_PER_5_OVER);
            if (((year == 2) && (speed >= 25) || (speed <= 29)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 2);
            if (((year == 2) && (speed >= 30) || (speed <= 34)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 3);
            if (((year == 2) && (speed >= 35) || (speed <= 39)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 3);
            if (((year == 2) && (speed >= 40) || (speed <= 44)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 4);
            if (((year == 2) && (speed >= 45) || (speed <= 49)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 5);
            if (((year == 2) && (speed >= 50) || (speed <= 54)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 6);
            if (((year == 2) && (speed >= 55) || (speed <= 59)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 7);
            if (((year == 2) && (speed >= 60) || (speed <= 64)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 8);
            if (((year == 2) && (speed >= 65) || (speed <= 69)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 9);
            if (((year == 2) && (speed >= 70) || (speed <= 74)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 10);
            if (((year == 2) && (speed >= 75) || (speed <= 79)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 11);
            if (((year == 2) && (speed >= 80) || (speed <= 84)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 12);
            if (((year == 2) && (speed >= 85) || (speed <= 89)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 13);
            if (((year == 2) && (speed >= 90) || (speed <= 94)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 14);
            if (((year == 2) && (speed >= 95) || (speed <= 99)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 15);
            if (((year == 2) && (speed >= 100) || (speed <= 104)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 16);
            if (((year == 2) && (speed >= 105) || (speed <= 109)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 17);
            if (((year == 2) && (speed >= 110) || (speed <= 114)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 18);
            if (((year == 2) && (speed >= 115) || (speed <= 119)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 19);
            if (((year == 2) && (speed >= 120) || (speed <= 124)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 20);
            if (((year == 2) && (speed >= 125) || (speed <= 129)))
                fine = INITIAL_FEE + (COST_PER_5_OVER * 21);
            else if (((year == 3) && (speed >= 16) || (speed <= 19)))
                fine = INITIAL_FEE + 50.00;
            if (((year == 3) && (speed >= 20) || (speed <= 24)))
                fine = INITIAL_FEE + 50.00 + (COST_PER_5_OVER);
            if (((year == 3) && (speed >= 25) || (speed <= 29)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 2);
            if (((year == 3) && (speed >= 30) || (speed <= 34)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 3);
            if (((year == 3) && (speed >= 35) || (speed <= 39)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 4);
            if (((year == 3) && (speed >= 40) || (speed <= 44)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 5);
            if (((year == 3) && (speed >= 45) || (speed <= 49)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 6);
            if (((year == 3) && (speed >= 50) || (speed <= 54)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 7);
            if (((year == 3) && (speed >= 55) || (speed <= 59)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 8);
            if (((year == 3) && (speed >= 60) || (speed <= 64)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 9);
            if (((year == 3) && (speed >= 65) || (speed <= 69)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 10);
            if (((year == 3) && (speed >= 70) || (speed <= 74)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 11);
            if (((year == 3) && (speed >= 75) || (speed <= 79)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 12);
            if (((year == 3) && (speed >= 80) || (speed <= 84)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 13);
            if (((year == 3) && (speed >= 85) || (speed <= 89)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 14);
            if (((year == 3) && (speed >= 90) || (speed <= 94)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 15);
            if (((year == 3) && (speed >= 95) || (speed <= 99)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 16);
            if (((year == 3) && (speed >= 100) || (speed <= 104)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 17);
            if (((year == 3) && (speed >= 105) || (speed <= 109)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 18);
            if (((year == 3) && (speed >= 110) || (speed <= 114)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 19);
            if (((year == 3) && (speed >= 115) || (speed <= 119)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 20);
            if (((year == 3) && (speed >= 120) || (speed <= 124)))
                fine = (INITIAL_FEE + 50.00) + (COST_PER_5_OVER * 21);
            else if (((year == 4) && (speed >= 16) || (speed <= 19)))
                fine = INITIAL_FEE + 100.00;
            if (((year == 4) && (speed >= 20) || (speed <= 24)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER);
            if (((year == 4) && (speed >= 25) || (speed <= 29)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 2);
            if (((year == 4) && (speed >= 30) || (speed <= 34)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 3);
            if (((year == 4) && (speed >= 35) || (speed <= 39)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 4);
            if (((year == 4) && (speed >= 40) || (speed <= 44)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 5);
            if (((year == 4) && (speed >= 45) || (speed <= 49)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 6);
            if (((year == 4) && (speed >= 100) || (speed <= 54)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 7);
            if (((year == 4) && (speed >= 55) || (speed <= 59)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 8);
            if (((year == 4) && (speed >= 60) || (speed <= 64)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 9);
            if (((year == 4) && (speed >= 65) || (speed <= 69)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 10);
            if (((year == 4) && (speed >= 70) || (speed <= 74)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 11);
            if (((year == 4) && (speed >= 75) || (speed <= 79)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 12);
            if (((year == 4) && (speed >= 80) || (speed <= 84)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 13);
            if (((year == 4) && (speed >= 85) || (speed <= 89)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 14);
            if (((year == 4) && (speed >= 90) || (speed <= 94)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 15);
            if (((year == 4) && (speed >= 95) || (speed <= 99)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 16);
            if (((year == 4) && (speed >= 100) || (speed <= 104)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER);
            if (((year == 4) && (speed >= 105) || (speed <= 109)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 18);
            if (((year == 4) && (speed >= 110) || (speed <= 114)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 19);
            if (((year == 4) && (speed >= 115) || (speed <= 119)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 20);
            if (((year == 4) && (speed >= 120) || (speed <= 124)))
                fine = (INITIAL_FEE + 100.00) + (COST_PER_5_OVER * 21);

            // finish coding this method

            return fine;
        }

        static void DisplayFine(double fine)
        {
            Console.WriteLine("Fine: {0:C}", fine);
        }

        static char GetUserChoice()
        {
            Console.Write
                ("\nPress \"C\" to [C]ontinue or \"Q\" to [Q]uit: ");
            string userEntry = Console.ReadLine();
            char choice = Convert.ToChar(userEntry);
            Console.WriteLine("------------------------------------");
            return choice;
        }
    }
}

I have a whole list of these statements going up to 125 mph and for different years: 1 through 4. I'm trying to make a program that takes input for speed of a vehicle, then gives appropriate ticket information according to speed.

The speed limit is 15 mph. For every 5 miles per hour over speed limit, $87.50 is added towards the total. Year 2 is sophomore so a $50.00 discount applies. Yet for like year 4, a $100.00 fee is added towards the total. I am getting same total for each speed. Why?

Answer 1

What others have said about operator precedence seems to be correct, but I think there is a bigger issue here. You really shouldn't need zillions of if-statements to model this problem. I'm sure you've found out by now that this method is not maintainable. Trying to change anything in that pile of ifs is going to be a real pain and very error prone.

First thing I would do is separate the calculation for the discount from the calculation for the fine. This way you do not have to manually calculate every possible combination of discount and fine. Here's what I'm talking about:

static double CalculateFine(int speed, int year)
{
    const double COST_PER_5_OVER = 87.5;
    const int SPEED_LIMIT = 15;
    const double INITIAL_FEE = 75;

    // Should there be a fine at all?
    if (speed > SPEED_LIMIT) {
        // The discount is 50 for each year, scaled so that year 1 is
        // -50, year 2 is 0, and so on.
        double discount = year * 50 - 100;

        // Now calculate the standard fee.
        int feeMultiplier = (speed - SPEED_LIMIT) / 5;
        double fine = feeMultiplier * COST_PER_5_OVER + INITIAL_FEE;

        return discount + fine;
    }
    return 0.;
}

In the end, the discount and fine are combined only one time. Really, this is just figuring out what formula is used to calculate the fine and then implementing that. If the fine were defined in a more arbitrary manner then perhaps a table would be useful.

Answer 2

Usually you would use a switch statement if an If statement has atleast one or more 'nested' If statements...

Answer 3

A table driven approach should work well here. Define the values in a table, then iterate over them with to find the one that matches, then use that for making the computation. This works well and you can easily add/edit/remove entries without touching the code - at all.

You can start with a simple statically allocated and defined table. If needed, you could then make the table load dynamically from another source, such as an XML file.

Answer 4

Well this isn't a direct answer (I think other people have covered that pretty well) you may want to check out the book Code Complete by Steve McConnell. It contains best practices to avoid code like you have above. It could help you out for future projects.

Answer 5

I think you're confusing your operator precedence and semantics.

First of all, you want to say (speed >=50) && (speed <= 54)

Second, in C the && has a higher precedence, so your expression as it is means: If (year==1 AND speed>=50) OR (if my speed is lower than 54)...

Third, you have an extra pair of () which suggests that you intended to prioritize things differently.

Finally, I'll mention that code that has a bunch of similar looking if statements is fairly poorly written. A better way to implement it would be either with tables or data structures to represent the ranges, or at least a better organization.

====

Update: Now that you've posted the code, it really looks like you're working too hard on this... First of all, if or switch based on year (there should only be about 4 years). For each year, call a separate function to do the calculation, that will already be cleaner. For example:

if(year==1) return calculateFineForYear1(speed);
if(year==2) return ...

(I don't know C#, but there should be some sort of switch statement)

Now you can manage each year separately. Since your conditions are in an ascending order of speed, you could do something like:

if(speed<50) return 0;
if(speed<55) return ...;
if(speed<60) return ...;
if(speed<65) return ...

It is still far from perfect, but it will already be a lot cleaner and manageable.

Now, looking at your ifs, it looks like you don't even need that many ifs, because you are paying 50 + C*diff, so why not just calculate the diff (speed-50) and then just return the result of the calculation?

For example:

Int total_above_limit = (speed – SPEED_LIMIT);
Int increments_of_5_above_limit = (total_above_limit)/5
Return ( (INITIAL_FEE – 50) + COST_PER_5_OVER*increments_of_5_above_limit))

(I don't know C#, I'm just guessing at the syntax)

Answer 6

Those if statements make my eyes bleed - and they're inconsistent as well. You have an error in the speed for the first range - being year 1 and having a speed of 15 would get you fined. In some you use a large line of if statements, in others if/elseif statements. There is a difference - especially when you're testing whether speed <= some number. (A bunch of if statements with your || or clause will each evaluate to true, an if/else statements will only evaluate the 1st one). You likely meant && (and) instead of || (or) - but you should replace those if statements for everybody's sanity. ;)

Figure out the formula, and code that - not the results. It looks like it's[1]:

    const double COST_PER_5_OVER = 87.50;
    const int SPEED_LIMIT = 15;
    const double INITIAL_FEE = 75.00;

    if (speed <= SPEED_LIMIT)
    {
         return 0;
    }

    double yearSurcharge;
    switch (year)
    {
        case 1:
           yearSurcharge = -50;
           break;
        case 2:
           yearSurcharge = 0;
           break;
        case 3:
            yearSurcharge = 50;
            break;
        case 4:
            yearSurcharge = 100;
            break;
        default:
            yearSurcharge = 0;
            break;
    }

    const int NUMBER_OF_FIVE_OVER = 21;
    int numberOfFiveOver = Math.Min((speed - SPEED_LIMIT) % 5; MAX_NUMBER_OF_FIVE_OVER)

    return INITIAL_FEE + yearSurcharge + (numberOfFiveOver * COST_PER_5_OVER);

which could be simplified to the following:

    if (speed > SPEED_LIMIT) {
         return INITIAL_FEE 
                + (year == 1 ? -50 : year == 3 ? 50 : year == 4 ? 100 : 0) 
                + Math.Min((speed - SPEED_LIMIT) % 5, MAX_NUMBER_OF_FIVE_OVER) * COST_PER_5_OVER;
    } else {
         return 0;
    }

though some will quibble over the nested ternaries for the year portion.

[1] Technically, you have a $0 fine for anything over 125 - but I don't think that's really what you wanted.

Answer 7

(speed >= 50) || (speed <= 54)

If you want this to mean "speed is between 50 and 54", your logic is faulty. You want and, not or.

I don't know what the etiquette is around here in supplying an actual answer as opposed to guiding someone there, but this is the kind of thing you should aim for. I haven't tested it personally though.

static double CalculateFine(int speed, int year)
{
    const double COST_PER_5_OVER = 87.50;
    const int SPEED_LIMIT = 15;
    const double INITIAL_FEE = 75.00;
    double fine = 0;

    if(speed <= SPEED_LIMIT)
    {
        return 0; // No fine imposed
    }

    fine = INITIAL_FEE;

    // Adjust for the different years
    switch(year) {
        case 1:
            fine -= 50;
            break;
        case 2:
            // nowt
            break;
        case 3:
            fine += 50;
            break;
        case 4:
            fine += 100;
    }

    // Add the remaining fine for each 5 miles over the limit
    // XXX: This is slightly different from yours, in that past 125,
    // it'll still keep adding COST_PER_5_OVER
    int perFiveOver = (int)Math.Floor((speed - SPEED_LIMIT) / 5);
    fine += (perFiveOver * COST_PER_5_OVER);

    return fine;

}

Answer 8

Not sure I'm 100% sure of what your code is trying to do, but is the problem down to the precedence of the logical operators i.e. should it be:

if (((year == 1) && ((speed >= 50) || (speed <= 54)))) fine = (INITIAL_FEE - 50.00) + (COST_PER_5_OVER * 7);

Note the extra brackets around the speed inequalities.

Answer 9

Hey, I'm a little confused, should it be

((year == 1) && (speed >= 50) && (speed <= 54))

?