-2

I Need someone pro/con this two approach which is better and why.

 if ((new String[] { "test1", "test2", "test3", "test4"}).Contains(MySelecredFooValue))
 {
 }

or

 if (MySelecredFooValue == "test1" ||
     MySelecredFooValue == "test2" ||
     MySelecredFooValue == "test3" ||
     MySelecredFooValue == "test4")
 {
 }

I nead to compare to approach my question not concentrate on Enums compare so I edited the question for getting better answer

Hamid
  • 577
  • 1
  • 6
  • 15

3 Answers3

1

The positive side of doing the first approach is that the code is more condensed and easier to add or remove values from the list of Foo values.

The negative side of doing the first approach is that it creates a new array object which requires more processing and memory overhead.

Josh Withee
  • 9,922
  • 3
  • 44
  • 62
1

IF you are just comparing those two options, then the second option (Multiple comparisons in a single "if") is more verbose, and will end up being more confusing to read. The O values are bon O(n) so the cyclical performance will be equivalent.

Whereas the first option is going to be compiled into an iterative process

foreach(value in array)
  if (value == finalvalue) 
    return true;

Since you dont see this codebehind, it is much cleaner, HOWEVER, you do create a new array, so memory is used.

If you value performance use option 2, if you value readability, use option 1. Mind you the performance effect will be negligible unless this process is being repeated many times without disposing of the array (Unlikely).

But if you are not tied to either of those implementations, really it looks like your best bet (Assuming real-world application is similar to the example) would be something like:

if(MySelectedFooValue != Foo.foo5)
{
}

Or even consider the integer representation of the enum:

if((int)MySelectedFooValue < 4)
{
}

Generally, when considering different comparisons, I first look at the O() value, then if an inverse comparison is simpler, then look at readability.

Noah Litov
  • 96
  • 4
1

Your first code sample will be slower and allocate more memory since:

new String[] { "test1", "test2", "test3", "test4"}

will create a new array on every invocation, and there is a (small) perf cost associated with iterating over an array.

You could offset the creation cost, by storing this array in a static field (i.e. just create it once):

private static string[] ValidValues = {"test1", "test2", "test3", "test4"};

Your second code sample will be faster, but also is more verbose (since MySelecredFooValue is repeated). Consider changing it to:

switch (MySelecredFooValue)
{
    case "test1":
    case "test2":
    case "test3":
    case "test4":
        //Your logic here
        break;
}

If you really like the array approach, and want better performance, try:

// declare this outside of your function
private static HashSet<string> ValidValues = new HashSet<string>() {"test1", "test2", "test3", "test4"};

// code for inside the function
if (ValidValues.Contains(MySelecredFooValue))
{
    //Your logic here
}

For small numbers of entries (like 3 or 4), performance of a HashSet often isn't better than an array - but if there are many more entries (e.g. 20) then usually a HashSet will substantially outperform an array for Contains calls.

mjwills
  • 23,389
  • 6
  • 40
  • 63