views:

150

answers:

9

What is the most terse syntax to combine a check for some preconditions with a switch statement? Can I combine an if/else and switch statement?

if (!IsValid(text))
{
   DoSomeLogging();
}
else
{
   switch (text)
   {
     case "1":
        DoSomething();
        break;
     case "2"
        DoSomethingElse();
        break;
     default:
        break;
   }
}

Edit: Removed skip comment. I wasn't looking to invert the statement.

+2  A: 
if (IsValid(text))
{
   switch (text)
   {
     case "1":
        DoSomething();
        break;
     case "2"
        DoSomethingElse();
        break;
     default:
        break;
   }
}
RedFilter
+6  A: 

I wouldn't want to combine them even if I could. I think it's a good idea to separate "validation" and "processing".

As a horribly grotty hack, you could do something like:

switch (Validate(text))
{
    case null:
        throw ArgumentException("Invalid text");
    case "1":
       DoSomething();
       break;
    case "2"
       DoSomethingElse();
       break;
    default:
        break;
}

... where Validate returns null if the text is invalid. (This assumes that null isn't a valid value to start with, of course.)

But that's horrible. I would prefer to see:

if (!IsValid(text))
{
    throw ArgumentException("Invalid text");
}
// No need for an else, just proceed with the switch from here
Jon Skeet
A: 

How about

if (!IsValid(text))
    return;
switch (text)
{
  case "1":
    DoSomething();
    break;
  ...
}

That is, just use early exit, no need to try and combine the two constructs.

unwind
A: 

If // Skip is really what it is, then just invert the condition:

if (IsValid(text))
{
   switch (text)
   {
     case "1":
        DoSomething();
        break;
     case "2"
        DoSomethingElse();
        break;
     default:
        break;
   }
}
Anton Gogolev
A: 

easy

 if (IsValid(Text)) 
    switch(text)
    { 
        // cases 

    }

or even terser, if IsValid returns text if text is valid, and null if it's not,

 private string IsValid(string text)
 {
      if (/* code to check if text is valid */)
         return text;
      else return null;
 }

then all you need is:

 switch(isValid(text))
 {
      //cases:
 }
Charles Bretana
A: 

I just noticed that you can combine an if/else and switch statement. Is this new to 3.5?

if (!IsValid(text))
   DoSomeLogging();
else switch (text)
{
  case "1":
     DoSomething();
     break;
  case "2"
     DoSomethingElse();
     break;
  default:
     break;
}
Even Mien
No, the `else` is just defined without a block, so it only applies to the next statement (just like `else if`).
bdukes
Do'h! Here I thought I had found out something new and interesting. Turns out it was something I just always avoided, since I tend to never leave out the curly braces, so I don't accidentally skip a statement if I need to add one.
Even Mien
+2  A: 

The "combined" else switch is nothing new:

if (!IsValid(text))
{
    DoSomeLogging();
}
else switch (text)
{
}

is the same as

if (!IsValid(text))
{
    DoSomeLogging();
}
else 
{
    switch (text)
    {
    }
}

This syntax has been inherited from C via C++.

ShellShock
A: 

A terse version could be:

switch (IsValid (text) ? text : null)
{
case null:
  DoSomeLogging ();
  break;
case "1":
  DoSomething();
  break;
case "2"
  DoSomethingElse();
  break;
default:
  break;
}

However, I never thought that switching on strings to be a particularly good idea. I don't know what code the compiler will generate here so can't really say anything about performance. If it's an occasional code path it's probably OK. If it's something called a lot then it's something to be wary about.

Skizz

Skizz
A: 

Switch statements are often a candidate for replacement with polymorphism (e.g., see here), which can result in more "elegant" (terse) code:

if (!myObject.IsValid())
{
   DoSomeLogging();
}
else
{
    myObject.ExecuteSomeMethod();
}

If you are worried about your code becomming too verbose, perhaps you should consider refactoring it into a class hierarchy?

ShellShock