Rewrite if else into switch

Question

I wrote some if-else statements like this:

   if (workedDays > 0 && workedDays <= 180)
   {
     x= 14;
   }

   if (workedDays > 180 && workedDays <= 540)
   {
     x= 28;
   }

   if (workedDays > 540 && workedDays <= 1068)
   {
     x= 42;
   }
   else
   {
     x= 56;
   }

I was trying to rewrite it as switch statement like this:

   switch (workedDays)
   {
     case (workedDays > 0 && workedDays <= 180):
       x=14;
       break;
     default:
       break;
   }

However, I am getting the error Cannot implicitly convert type 'bool' to 'int'.

How can I fix it?

workedDays is an int and your switch expressions evaluate to bools, so obviously this is not working. I think IF statements are better and more clear in this case, don't use switch if there is no real benefit. — L-Four, Apr 03 '13 at 12:13
switch can only use constants in C#. Not expressions like VB.Net. What is wrong with your if:s? — adrianm, Apr 03 '13 at 12:14

score 6 · Accepted Answer · answered Apr 03 '13 at 12:16

6

While the switch(true) answer of @Jeroen might work, I suggest the following for readablity:

if (workedDays <= 180)
    x = 14;
else if (workedDays <= 540)
    x = 28;
else if (workedDays <= 1068)
    x = 42;
else 
    x = 56;

With the else if construction, there is no need to double check both minimum and maximum values.

answered Apr 03 '13 at 12:16

w5l

5,341
1
25
43

I upvoted but would like to point out that you're missing the case where `workedDays <= 0`... though it seems that might not be correct in the OPs question? – paul Apr 03 '13 at 12:18
@Williem Duncan. Thank you. I was trying to write it for purpose of readability. Nothing wrong in my if-else stamtent was. Just wanted to learn this way. – coder Apr 03 '13 at 12:20
@paul I removed that for readability (and I also assumed it might be wrong to get `56` when `workedDays < 0`), but it's easily added. – w5l Apr 03 '13 at 12:24

Skizz · Answer 2 · 2013-04-03T15:19:15.457

2

Well, I personally dislike having so many if-else-if's.

I usually follow the idea "three or more, use a for!"

So, I'd do this:-

class Program
{
  struct Data
  {
    public Data (int l, int u, int v)
    {
      lower = l;
      upper = u;
      value = v;
    }
    public int lower;
    public int upper;
    public int value;
  }

  static void Main (string [] args)
  {
    Data []
      // at the moment, this is coded into the source
      // it can be read from a file at run time instead
      data = { new Data (0, 180, 14), new Data (180, 540, 28), new Data (540, 1068, 42) };

    int
      // again, the default case is hard coded, it too can be loaded from a file
      x = 56,
      workedDays = 100;

    foreach (Data item in data)
    {
      if (workedDays > item.lower && workedDays <= item.upper)
      {
        x = item.value;
        break;
      }
    }
  }
}

You can then fill in the data array from a file so the search becomes run-time configurable so you don't need to rebuild the program to change the ranges and results.

edited Apr 03 '13 at 15:19

answered Apr 03 '13 at 12:24

Skizz

69,698
10
71
108

This makes it into a horrible mess, would be a lot more useful with key-value pairs of maximum value and result, or if minimum and maximum is required, a collection of tuples. – w5l Apr 03 '13 at 12:26
What about readability? This feels like obfuscating the code. – L-Four Apr 03 '13 at 12:27
@WillemDuncan: well, yes, I did allude to that in the answer if you read the comments. I was being brief and sort of language agnostic (and short on time). Feel free to replace the array of ints with array of structures. – Skizz Apr 03 '13 at 12:28
@Skizz. I like your solution. My whole goal was to write something more readeable. With lot of if-else statements in code you have to look all of them one by one in order to determine them.And your solution is good for that purpose. – coder Apr 03 '13 at 12:31
@WillemDuncan: Updated the source, now uses a struct with named data rather than the anonymous int array. – Skizz Apr 03 '13 at 15:20
@Skizz. Would it be even better with following code: `public struct Data { public int Lower { get; set; } public int Upper { get; set; } public int Value { get; set; } }` and then `Data[] data = {new Data{Lower=0,Upper=180,Value=14}, new Data{Lower=180,Upper=540,Value=28}, new Data{Lower=540,Upper=1068,Value=42} };` – coder Apr 03 '13 at 21:31
@black123: yes, that is a good solution, but does require newer versions of C#. I was trying to get the idea across rather than the implementation since the idea works equally well in most languages. – Skizz Apr 04 '13 at 01:19

JMan · Answer 3 · 2013-04-03T12:18:41.697

0

do:

switch (true)
    {
        case (workedDays > 0 && workedDays <= 180):
            x=14;
            break;
        default:
            break;
    }

edited Apr 03 '13 at 12:18

answered Apr 03 '13 at 12:13

JMan

2,611
3
30
51

3

This really shouldn't be done, to switch on true not only makes no sense but looks wrong. – Clint Apr 03 '13 at 12:14
Not saying this is the best way but this is the way to fix the error :), thats what he asked. – JMan Apr 03 '13 at 12:18
1

I have never seen this approach before and did a quick search on it and in some cases, including readability it seems to make sense. http://stackoverflow.com/questions/2765981/javascript-switchtrue `"Yes --- I think that you're being overly dogmatic about it, and I see a readability tradeoff either way. The switch statement gives me a visual cue that none of the following conditions will be evaluated, where with the series of ifs I have to notice the returns to realize that. I think switch(true) is a good and useful idiom and shouldn't be rejected, though this exact usage is not idea"` -linked – Scott Apr 03 '13 at 12:22
Does this compile in C#? I tried with LinqPad, and get the following error: `A constant value is expected` – Arturo Torres Sánchez Oct 10 '14 at 22:49

score 0 · Answer 4 · answered Apr 03 '13 at 12:14

If you consider what you're asking the compiler to do you'll see why you've got the problem.

You're switching on workedDays which is presumably an int:

switch(workedDays:int)
{
   case (workedDays > 0 && workedDays <= 180):

   break;
}

Where you've got your case that is evaluated into a true or false result (a bool), so you're asking it to choose between integers but giving it a bool as a possibility!

score 0 · Answer 5 · edited May 23 '17 at 11:44

0

You can't. Switch statements can only be used with compile-time constants. See this question

edited May 23 '17 at 11:44

Community

1
1

answered Apr 03 '13 at 12:16

Alex

7,639
3
45
58

Rewrite if else into switch

5 Answers5