Is it a bad practice to make this comparison?

As I always repeat, this good or bad practice depends on context. In this case without knowing all the requirements of what you are doing you have no way to respond. Both are correct and acceptable.

If you make the second to get shorter, more funny, in thesis save typing, then do not do, it does not make sense. Especially in C # which privileges compilation time.

The second one passes a run-time processing to resolve something that could have been resolved at compile time. Leave Contains() to cases where you do not know what the data list is, or it can be changed frequently, or if it is too large, which does not seem to be close at all.

If C # had some optimization that would guarantee that Contains() would be linearized and then unrolled, which probably would generate more or less the same code as the first, there could even do. It has language that has an operator for this which generates an optimized code.

But if you have no worries about runtime, you can do it, though I do not recommend it.

But you have to analyze the context, if it makes sense to produce a list, because this is what you are manipulating, which does not seem to be the case, make a list. If it is not a list, the most readable is the first. If it's a list, I do not know if it should be created there in if , probably it should be somewhere else. Do what you want best, what ever is the intention.

I think it is preferable to use the second approach (using the Contains method).

There is a big gain in readability and maintainability of the code.

In particular, I'd rather declare the array / list before using it in the if condition. I believe this way the readability of the code gets even better. This way:

var lista = new List<string> { "abc", "54", "23A", "3xe", "123" }

if (lista.Contains(foo)) { }

Another alternative (not very good, in my opinion) would be to use switch case . Something like:

  switch (foo)
  {
      case "abc":
      case "54":
      case "(etc...)":
          Console.WriteLine("Valor de foo:" + foo);
          break;
      default:
          Console.WriteLine("Foo não está especificado em nenhum case");
          break;

  }

As for performance, it's practically imperceptible to human eyes, maybe it's benchmarking tests. But unless you're writing code to support life (medicine area) or another area where response time is critical, that's fine.

Based on this response , you can write an extension method that emulates the conditional < strong> In that we normally use in SQL.

E.g .: Select * from Produtos Where Id In (1,2,3,4,5);

I particularly find it prettier than the syntax of Contains that has to go through the list with the possibilities first, but it's a matter of taste.

This would be the extension method:

public static bool In<T>(this T obj, params T[] args)
{
    return args.Contains(obj);
}

Usage:

if(foo.In("xyz", "abc", "123"))
{
   // ...
}

I think it's not a bad practice, just a way to organize and make the code a little easier to understand. If the amount of possibility is too large I would put it in a separate object.

var listaFoo = new List<string> { "abc", "54", "23A", "3xe", "123", "abc1", "154", "23A2", "34xe", "223" };
if (listaFoo.Contains(foo)) { }

For cases with few elements, it can be declared in the same condition:

if({"abc", "23A", "3xe", "123"}.Contains(foo)) 
{

Another way, but a little bigger and worse is:

switch(foo)
{
  case "abc":
  case "bcd":
  case "cdf":
    //do something
    break;
  case "xyz":
    //do something else
    break;
  default:
    //do a different thing
    break;
}

Now if performance is very important to you, this is the way with the least processing steps:

if(foo == "abc" || foo == "54" || foo == "23A" || foo == "3xe" || foo == "123")