3

I have a function like this(foo): I need to compare the input string and perform a task accordingly . Task is same, but only for a selected set of values. For all other values do nothing.

function foo(string x)

{
if(x == "abc")
    //do Task1

if(x == "efg")
    //do Task1
if(x == "hij")
    //do Task1
if(x == "lmn")
    //do Task1
}

Is there any other means to do checking other than this? Or putting OR operator inside if?

What is the preferred way?

BenMorel
  • 34,448
  • 50
  • 182
  • 322
TutuGeorge
  • 1,972
  • 2
  • 22
  • 42
  • 4
    The [`switch`](http://msdn.microsoft.com/en-us/library/vstudio/06tc147t.aspx) statement is a good fit for such cases; it allows you to write the target values nicely separated and with a minimum of unnecessary boilerplate. – Jon Aug 05 '13 at 13:34
  • A switch statement doesn't really make much of a difference here IMO. – James Aug 05 '13 at 13:35
  • Enter your comparisons in to a list or collection and iterate over that collection and check inside the for loop and track which collection entry matches. Also I would not do == on string comparisons I would use the .Equals extension method – Bearcat9425 Aug 05 '13 at 13:37
  • 1
    Some people are partial to `if (new[] {"abc", "efg", "hij", "lmn"}.Contains(x)) { Task1(); }`... Not sure myself, but it works. – anaximander Aug 05 '13 at 13:39
  • The hashset lookup is very nice because it lends itself to easy refactoring; once you realize you should have data-driven it in the first place. – Rake36 Aug 05 '13 at 13:40
  • 1
    @anaximander Good one. But it is interesting that `string[]` does not actually contain a method `Contains`, so that works only because of Linq. Not that that is a problem. The pure array solution would be `if (Array.IndexOf(new[] {"abc", "efg", "hij", "lmn"}, x) != -1) { ... }`, but that looks quite awful. – Jeppe Stig Nielsen Aug 05 '13 at 13:51
  • 1
    Are you just looking for some nice in-line syntax? If so, you can always make an extension method that gives you syntax like `if(x.Is("abc", "efg", "hij", "lmn")) { }` – Chris Sinclair Aug 05 '13 at 13:55
  • @JeppeStigNielsen Indeed. I should perhaps have mentioned that it requires Linq, but I build database-driven software so I'm so used to having it available. Come to think of it, I can't remember the last C# program I saw that didn't use it somewhere... – anaximander Aug 05 '13 at 13:56
  • Thanks to everyone. Since array of string doesnt have a "contains" method by itself, i should define an extension method myself or include LINQ namespace. Otherwise i have to use "List"(collections). But i have a list of 10 strings to compared. At max it can grow to 50 strings which is a rare possiblity. In that is it better to stick with array of strings? – TutuGeorge Aug 06 '13 at 05:20
  • @anaximander Doenst your solution require extension methods of Linq. To make use of lambda function. Like the solution though:) – TutuGeorge Aug 06 '13 at 05:22

4 Answers4

10

There are many ways of doing it. One would be as follows:

var target = new HashSet<string>{ "abc", "efg", "lmn" };
if (target.Contains(x)) {
    ...
}

At max [my list of strings] can grow to 50 strings which is a rare possibility.

Then you should make target a static readonly in your class, like this:

private static readonly StringTargets = new HashSet<string>{ "abc", "efg", "lmn" };

Doing so would ensure that the set is created only once, and is not re-created each time the execution goes through the method that uses it.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • +1 I'd say this is a better alternative to the `if` than the `switch`, with the `switch` you are still pretty much writing the same amount of code. – James Aug 05 '13 at 13:41
  • Agreed, also depending on the amount of comparison entries you are putting in there it could be faster to go with a List VS a hashset, but as your list of comparisons grows I think hashset offers better performance. see this post here. http://stackoverflow.com/questions/150750/hashset-vs-list-performance – Bearcat9425 Aug 05 '13 at 13:44
  • I think the nicest thing about this is that it lets you easily move `target` into some class or provider and do `if(BusinessRulesFooClass.Something(x))`. In fact, depending on the scenario it's possible it should have been like that all along... – anaximander Aug 05 '13 at 13:58
  • Since array of string doesnt have a "contains" method by itself, i should define an extension method myself or include LINQ namespace. Otherwise i have to use "List"(collections). But i have a list of 10 strings to compared. At max it can grow to 50 strings which is a rare possiblity. In that is it better to stick with array of strings? – TutuGeorge Aug 06 '13 at 05:23
  • `using System.Linq` and you're good to go. No custom extensions necessary. – anaximander Aug 06 '13 at 07:49
  • @NOOB If the array could grow to 50, then you should definitely keep the hash set: it is going to be significantly faster - I'd say about ten times or more, depending on the exact strings. Make the hash set a `static readonly`. – Sergey Kalinichenko Aug 06 '13 at 09:33
  • 50 is very rare possiblity. In the near immediate future even 10 is imporbable. – TutuGeorge Aug 06 '13 at 09:39
  • 1
    @NOOB You should still get about three times speed improvement with ten strings. Making it static ensures that the set object is not re-created each time the code goes through the method. Making it readonly is a way to show to the readers of your code that you are not planning to replace the instance of the set after initialization. This enables additional optimizations in the compiler. – Sergey Kalinichenko Aug 06 '13 at 09:43
  • @dasblinkenlight Thanks. This seems like a perfect fit for the situation. – TutuGeorge Aug 06 '13 at 09:44
  • @dasblinkenlight . I am not able to make the hashset variable a private static readonly. The compiler is showing errors. Please advice – TutuGeorge Aug 06 '13 at 11:37
  • 1
    @NOOB What error do you get? As any `static` variable, it needs to be defined outside of the method, and its declaration may not use `var`. Copy-paste the second code snippet from the answer outside your method, and it should work. I assume that you have `using System.Collections.Generic;` at the top of your file, too. – Sergey Kalinichenko Aug 06 '13 at 12:11
7

do it like this

function foo(string x)
{
  switch(x)
  {
      case "abc":
      case "efg":
      case "hij":
      case "lmn":
        {
          //do task 1
          break;
        }
      default:
        break;
  }
}

Alternatively you can do this

if(x == "abc"||x == "efg"||x == "hij"||x == "lmn")
   //do Task1
Ehsan
  • 31,833
  • 6
  • 56
  • 65
  • Implementation of switch case in this fashion might be a better solution for me i think. – TutuGeorge Aug 06 '13 at 05:56
  • Will this cause the comparsion operation to be done against all the cases? Rather than do the task on finding the first occurence. – TutuGeorge Aug 06 '13 at 05:58
0

One way would be to make an array of the acceptable strings, then see if that array contains x

function foo(string x)

{


     string[] strings = new string[] {"abc", "efg", "hij", "lmn"};

     if (strings.contains(x)){
        //do task 1
     }
}
Dan Drews
  • 1,966
  • 17
  • 38
-1

You can use a switch statement with a default value to catch anything that didn't match

http://blogs.msdn.com/b/brada/archive/2003/08/14/50227.aspx

function foo(string x) {

    switch(x) {

     case "abc":
      //do task 1
     break;

     case "efg":
      //do task 2
     break;

     default:
      //all other cases
     break;
    }
}
Bam4d
  • 610
  • 3
  • 10