1

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?

3
  • 1
    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. Commented Apr 3, 2013 at 12:13
  • 2
    switch can only use constants in C#. Not expressions like VB.Net. What is wrong with your if:s? Commented Apr 3, 2013 at 12:14
  • 'If' is more suitable in this situation Commented Apr 3, 2013 at 12:15

5 Answers 5

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.

Sign up to request clarification or add additional context in comments.

3 Comments

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?
@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.
@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.
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.

7 Comments

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.
What about readability? This feels like obfuscating the code.
@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. 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.
@WillemDuncan: Updated the source, now uses a struct with named data rather than the anonymous int array.
|
0

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!

Comments

0

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

Comments

0

do:

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

4 Comments

This really shouldn't be done, to switch on true not only makes no sense but looks wrong.
Not saying this is the best way but this is the way to fix the error :), thats what he asked.
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. 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
Does this compile in C#? I tried with LinqPad, and get the following error: A constant value is expected

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.