Cleaning up a three-factor if statement

Question

A method that I'm working on in a C# project includes the following:

if (isDailyRateRequired) 
{
    CalculatedDailyRate = dailyRate;
}

if (isWeeklyRateRequired) 
{
    CalculatedDailyRate = weeklyRate / 7;
}

if (isMonthlyRateRequired) 
{
    CalculatedDailyRate = monthlyRate / 30;
}

There has to be a cleaner implementation than this. I could compress it to one line with a set of nested ternary operators, but that would be pretty much unreadable.

Other suggestions?

Answer 1

By looking at your code it appears to me that your basic problem is that isDailyRateRequired, isWeeklyRateRequired and isMonthlyRateRequired are distinct boolean variables instead of an enum. If you just turned them all into an enum things would be a lot more simple, there would be no possibility of inconsistencies between their values, and you would not even have this multiple-choice issue.

Answer 2

Since those seem to be mutually exclusive conditions, you may not want to execute all 3 ifs every time by putting an else in between:

if (isDailyRateRequired) 
{
    CalculatedDailyRate = dailyRate;
}
else if (isWeeklyRateRequired) 
{
    CalculatedDailyRate = weeklyRate / 7;
}
else if (isMonthlyRateRequired) 
{
    CalculatedDailyRate = monthlyRate / 30;
}

Answer 3

If these conditions are exclusive conditions you should not use booleans. Use an enumeration with three named constants (or states): DailyRate, WeeklyRate and MonthlyRate. It prevents invalid states where more than one (or zero) boolean are true. (As @MikeNakis already suggested.)

If these three if statements repeating in the code replace the conditional with polymorphism. (See Replacing the Conditional Logic on Price Code with Polymorphism in Refactoring: Improving the Design of Existing Code by Martin Fowler).

Since these booleans comes from a legacy database maybe you want to throw an exception or at least log an error message when more than one (or zero) flags are true.

Answer 4

How about adding a calculated column to your DataTable?

The expression is not exactly simpler, but doing it this way might provide it's own advantages:

new System.Data.DataColumn("CalculatedDailyRate", typeof(decimal), "IIF(isDailyRateRequired, dailyRate, IIF(isWeeklyRateRequired, weeklyRate / 7, IIF(isMonthlyRateRequired, monthlyRate / 30, null)))")

Multi-line version that might be easier to read:

        new System.Data.DataColumn("CalculatedDailyRate", typeof(decimal), @"
            IIF(isDailyRateRequired, dailyRate,
            IIF(isWeeklyRateRequired, weeklyRate / 7,
            IIF(isMonthlyRateRequired, monthlyRate / 30,
            null)))")

Answer 5

You could create a class Interval with subclasses DailyInterval, WeeklyInterval, and MonthlyInterval and then do:

interval = isDailyRateRequired? new DailyInterval(dailyRate):
           isWeeklyRateRequired? new WeeklyInterval(weeklyRate):
           isMonthlyRateRequired? new MonthlyInterval(monthlyRate):
           /* throw exception */;

calculatedDailyRate = interval.DailyRate();

...where the method DailyRate() automatically returns the appropriate rate based on the internally-known interval duration.

Of course, this only makes sense if you'll be using intervals in lots of places.