4
\$\begingroup\$

I have this if statement

if (dt.Month > 3 && dt.Month < 11)
{
    if (hours == 23 || (hours >= 0 && hours <= 6))
    {
        session = String.Empty;
    }
    else if (hours == 7)
    {
        session = String.Empty;
    }
    else if (hours >= 8 && hours <= 11)
    {
        session = String.Empty;
    }
    else if (hours >= 12 && hours <= 15)
    {
        session = String.Empty;
    }
    else if (hours >= 16 && hours <= 20)
    {
        session = String.Empty;
    }
    else if (hours == 22)
    {
        session = String.Empty;
    }
    else
    {
        session = String.Empty;
    }
}
else
{
    if (hours == 22)
    {
        session = String.Empty;
    }
    else if (hours == 23 || (hours >= 0 && hours <= 6))
    {
        session = String.Empty;
    }
    else if (hours == 7)
    {
        session = String.Empty;
    }
    else if (hours >= 8 && hours <= 12)
    {
        session = String.Empty;
    }
    else if (hours >= 13 && hours <= 16)
    {
        session = String.Empty;
    }
    else if (hours >= 17 && hours <= 20)
    {
        session = String.Empty;
    }
    else if (hours == 21)
    {
        session = String.Empty;
    }
    else
    {
        session = String.Empty; // I dont think this one is needed
    }
}

As you can see it's pretty long and nasty. Is there a way to shrink it? I was going to use a switch but in C# you can't fall through case statements.

\$\endgroup\$
  • 1
    \$\begingroup\$ Easy. if (dt.Month > 3 && dt.Month < 11) { sesion = String.Empty; }. I think you can fall through case statements in C# depending what you are trying to do, maybe post real code. \$\endgroup\$ – JohnMark13 Sep 24 '13 at 13:20
  • \$\begingroup\$ Why not using a array of triple of <rangMin, rangMax, session>, and go though it using an loop. \$\endgroup\$ – ZijingWu Sep 24 '13 at 13:32
  • 4
    \$\begingroup\$ What's stopping you from just doing session = String.Empty;? If the value is actually different for each branch, then you should have made that clear in your code. \$\endgroup\$ – svick Sep 24 '13 at 14:11
2
\$\begingroup\$

Shouldn't you replace the whole code with session = String.Empty; ?

Let's assumme that this is just a placeholder for actual code. If you handle your different cases in order and if hours is an int, then you can make the logic a bit easier to follow :

if (hours == 23 || (hours >= 0 && hours <= 6))
{
    session = A;
}
else if (hours == 7)
{
    session = B;
}
else if (hours >= 8 && hours <= 11)
{
    session = C;
}
else if (hours >= 12 && hours <= 15)
{
    session = D;
}
else if (hours >= 16 && hours <= 20)
{
    session = E;
}
else if (hours == 22)
{
    session = F;
}
else
{
    session = G;
}

becomes

if (hours < 0 || hours > 23)
{
    session = G;
}
else if (hours <=6)
{
    session = A;
}
else if (hours <= 7)
{
    session = B;
}
else if (hours <= 11)
{
    session = C;
}
else if (hours <= 15)
{
    session = D;
}
else if (hours <= 20)
{
    session = E;
}
else if (hours <= 22)
{
    session = F;
}
else
{
    session = A;
}
\$\endgroup\$
2
\$\begingroup\$

Is there a way to shrink it?

Yes. With the specific code you wrote, the equivalent code is:

session = String.Empty;

In case you were not referring to the value but the chained/nested if structure (i.e. if in your actual application you use different values in the different branches of the nested if), you can abstract each branch of your if into an object/closure and treat them as an iterable sequence of conditions.

Your objects should probably respect this interface:

interface ICondition {
    bool Applies(your_parameters_here);
    string ResultValue { get; }
}

Then you would have an array of instances of ICondition specializations and iterate through it, until you get one which evaluates Applies() to true, set the value to ResultValue and break the loop.

Advantages:

  • your XXX lines of chained if statements become a simple for loop.

  • it will be easy to extend (just add another specialization/instance of ICondition to your conditions array)

Disadvantages:

  • it will probably be slower than the if/else if construct. This only matters if your condition block is in a critical part of your code (and if you have chained ifs in critical parts of your code, your problem should probably be solved by rethinking your application flow).
\$\endgroup\$
1
\$\begingroup\$

"in C# you can't fall through case statements"

Yes, you can, as long as there is no code in them.

if (dt.Month > 3 && dt.Month < 11) {
  switch (hours) {
    case 23:
    case 0:
    case 1:
    case 2:
    case 3:
    case 4:
    case 5:
    case 6:
      session = String.Empty; break;
    case 7:
      session = String.Empty; break;
    case 8:
    case 9:
    case 10:
    case 11:
      session = String.Empty; break;
    case 12:
    case 13:
    case 14:
    case 15:
      session = String.Empty; break;
    case 16:
    case 17:
    case 18:
    case 19:
    case 20:
      session = String.Empty; break;
    case 22:
      session = String.Empty; break;
    default:
      session = String.Empty; break;
  }
} else {
  switch (hours) {
    case 22:
      session = String.Empty; break;
    case 23:
    case 0:
    case 1:
    case 2:
    case 3:
    case 4:
    case 5:
    case 6:
      session = String.Empty; break;
    case 7:
      session = String.Empty; break;
    case 8:
    case 9:
    case 10:
    case 11:
    case 12:
      session = String.Empty; break;
    case 13:
    case 14:
    case 15:
    case 16:
      session = String.Empty; break;
    case 17:
    case 18:
    case 19:
    case 20:
      session = String.Empty; break;
    case 21:
      session = String.Empty; break;
    default:
      session = String.Empty; break;
  }
}
\$\endgroup\$
1
\$\begingroup\$

I was going to use a switch but in C# you can't fall through case statements.

Until recently I would have agreed with this statement. But actually you can do fall-through in C#, you just have to do it manually.

Each case in the switch statement is a special label that you can jump to with a goto case statement. The labels are scoped to the switch statement, so you can't jump into the middle of the switch from outside.

In your case you can use the form that Guffa posted, but for future reference this also works:

switch (hours)
{
    case 0:
    case 1:
    case 2:
    case 3:
        bIsEarly = true;
        goto case 4;
    case 4:
    case 5:
    case 6:
        session = String.Empty;
        break;
   // ...and so on
}

The goto case statement is more versatile than classic fall-through because it gives you the ability to do conditional fall-through and so on:

switch (hours)
{
    case 0:
    case 1:
    case 2:
    case 3:
        bIsEarly = true;
        if (dt.Month == 1)
            goto case 4;
        session = String.Empty;
        break;
    case 4:
    case 5:
    case 6:
        session = String.Empty;
        break;
   // ...and so on
}

The downsides of course are the same as for other goto usage, including the howls of rage you'll hear from the rabid anti-goto contingent. Sometimes it's useful, sometimes not. Don't go overboard.

\$\endgroup\$
  • 1
    \$\begingroup\$ I've never seen a compiler written without goto statements :p \$\endgroup\$ – dfhwze Jun 23 at 3:44
  • \$\begingroup\$ It's not required, but there are certain cases (no pun intended) where it is useful. These days we hide goto behind other control structures like continue and break because they avoid the common problems with goto. \$\endgroup\$ – Corey Jun 23 at 22:35
  • \$\begingroup\$ I'm not blindly against goto, but I always get nuked by the team when I argue the case for them. \$\endgroup\$ – dfhwze Jun 24 at 4:24
  • \$\begingroup\$ @dfhwze Yeah, I know. It's because everyone knows that goto is evil and must be avoided at all costs. Even if it means writing horrible code to achieve that goal. Sure, goto-ridden code is a pain in the butt, but if it's the best option then use it. It's not like any .NET language will let you abuse it the way people used to with C. \$\endgroup\$ – Corey Jun 25 at 0:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.