Home > Software engineering >  Is it possible to optimize these if-else statements?
Is it possible to optimize these if-else statements?

Time:10-11

I am new to C# and curious if there is some kind of better structure for this code:


This code determines the real month from Czech Birth Number = personal identification number in the Czech republic. Numbers there are constant.

private ushort RawMonth
// get raw integer month code without resolving it
{
    get { return Convert.ToUInt16(SanitizedBirthNumberString.Substring(2, 2)); }
}


if (RawMonth >= 01 && RawMonth <= 12)
{
    return RawMonth;
}
else if (RawMonth >= 51 && RawMonth <= 62)
{
    return RawMonth - 50;
}
else if (RawMonth >= 21 && RawMonth <= 32)
{
    return RawMonth - 20;
}
else if (RawMonth >= 71 && RawMonth <= 82)
{
    return RawMonth - 70;
}
else
{
    // just some of my custom exceptions
    throw new MonthCodeInvalidException();
}

Is it just me? Because those if-else statements look deprecated. Thank you.


I had to make a few major adjustments, starting with setting the C# language version to 9.0.

But eventually, I have this code, it compiles, I just never used such syntax, please check after me, thank you.

public ushort ResolvedMonth
{
    get => RawMonth switch
    {
         >= 01 and <= 12 => RawMonth,
         >= 21 and <= 32 => (ushort)(RawMonth - 20),
         >= 51 and <= 62 => (ushort)(RawMonth - 50),
         >= 71 and <= 82 => (ushort)(RawMonth - 70),
         _ => throw new MonthCodeInvalidException()
    };
}

Interestingly enough, for it to run, I had to use typecast, have no idea why... But anyway - looks much neater.

Note: It uses C# 9.0 introduced Relational patterns (link to MS C# reference).

CodePudding user response:

If you want them hard coded, and your inputs generally won't have bad values in, you could pattern match in increasing order:

RawMonth switch
{
  <=12 => RawMonth,
  <=32 => RawMonth-20,
  <=62 => RawMonth-50,
  <=82 => RawMonth-70,
  _ => throw ...
}

If you'll have bad input and need the ranges:

RawMonth switch
{
  (>=1) and (<=12) => RawMonth,
  (>=21) and (<=32) => RawMonth-20,
  (>=51) and (<=62) => RawMonth-50,
  (>=71) and (<=82) => RawMonth-70
    _ => throw ...
}

The parentheses are optional; I find it helps readability but your opinion may vary!

It's a shame that the number pattern isn't more consistent (eg if the 20 range actually started at 30), as you could have just eg modded them by 20, but I suppose you could decrement by 10 or 20 until they come into range

if(RawMonth>40)
  RawMonth -= 10;

while(RawMonth>12)
  RawMonth -= 20;

if(RawMonth < 1)
  throw ...

return RawMonth;

I think that's the logic (though I didn't put any >82 check in)

CodePudding user response:

In principle there is nothing wrong with if .. else if .. else (for a few cases like yours). But you can also create a list of tuples with the lower and upper bounds and the respective decrement value and then search for the correct tuple in that list.

//create a list with lower and upper limits and the respective decrement value
var limits = new List<(int, int, int)> {
    (1, 12, 0),  //lower limit, upper limit, decrement value
    (21, 32, 20),
    (51, 62, 50),
    (71, 82, 70)
};
    
var rawmonth = 23;
//find the respective interval for your given rawmonth
var d = limits.FirstOrDefault(x => x.Item1 <= rawmonth && x.Item2 >= rawmonth);

//if nothing is found, d will be (0,0,0)
if (d.Item1 == 0)
  throw new YourCustomException();

//substract the respective decrementvalue
rawmonth -= d.Item3;

CodePudding user response:

I suggest naming the integers if they have meanings, to don't have magic numbers that are hard to read and understand by others

When I find myself with something similar, I put the conditions in delegates in a Dictionary and in the method I try to get the matching by Key and return the Value from Dictionary if nothing exist return the default

I have attached a link that demonstrates my point Avoid If Statements

CodePudding user response:

Use a switch statement as mentioned here or here.

The first link's style:

switch (RawMonth)
{
    case int n when n >= 01 && n <= 12:
        ...
        break;
    case int n when n >= 51 && n <= 62:
        ...
        break;
    ...
}

CodePudding user response:

The first thing if-else statement is not depreciated. You can keep it as it is. You can also use a switch statement with expression.

    int output = RawMonth switch
            {
                int Month when Month >= 01 && Month <= 12 => Month,
                int Month when Month >= 51 && Month <= 62 => Month - 50,
                int Month when Month >= 71 && Month <= 82 => Month - 70,
                _ => throw new Exception("th")
            };
return output ;

Above code can be even simplified to

        int output = RawMonth switch
        {
            >= 01 and <= 12 => RawMonth,
            >= 51 and  <= 62 => RawMonth - 50,
            >= 71 and  <= 82 => RawMonth - 70,
            _ => throw new Exception("th")
        };
  • Related