10

I just wrote an if statement in the lines of

if (value == value1 || value == value2 || value == value3 || value == value4)
    //do something

and it annoys me that I always have to repeat the 'value ==' part. In my opinion this is serving no purpose other than making it difficult to read.

I wrote the following ExtensionMethod that should make above scenario more readable:

public static bool IsEqualToAny<T>(this T value, params T[] objects)
{
    return objects.Contains(value);
}

Now I can simply write

if (value.IsEqualToAny(value1, value2, value3, value4))
    //do something

Is this a good usage of an ExtensionMethod?

EDIT:

Thanks for all the great answers. For the record: I have kept the method. While the suggestion that you could simply use new []{value1,value2,value3,value4}.Contains(value) is true, I simply prefer reading this kind of if statement from left to right (if this value is equal to any of these instead of if these values contain this value). Having one more method show up in intellisense on each object is not an issue for me.

8 Answers 8

5

It is unusual to write an extension method for an unrestricted T. Not least, this approach will quickly make your intellisense quite hard to use.

While valid, I'd probably avoid this as an extension method - perhaps just use a standard static utility method.

The C# 3 array initializer syntax might be easier?

bool isTrue = new[] { 1, 2, 3 }.Contains(3);

Of course, for large sets of data, you might want to cache a HashSet<T> somewhere ;-p

4
  • thanks for the comment Marc. could you explain why this will make intellisense hard to use? Feb 11, 2009 at 9:24
  • Because it'll appear on every single variable when you press ".". It only takes a handful of such methods to start making it hard to find the methods relevant to a type... in reality, you aren't going to want to use this method that often (I hope) compared to every other method available. Feb 11, 2009 at 9:26
  • I agree - having extension methods for objects or unrestricted generic type parameters is just one step away from having abominations in your code like null.MyExtensionMethod() Feb 11, 2009 at 12:14
  • I won't mention the "ThrowIfNull<T> where T : class" extension method (for arg checking), then ;-p Feb 11, 2009 at 13:04
4

You haven't added functionality that is only useful to a specific application or context, your extension is clearly named and the behaviour is obvious without having to look at the implementation.

The answer is "Yes, it is"

1

Looks good to me, although it seems a little unconventional.

1

It seems pretty fair, but I'd take a step back. Can you put any business meaning in the comparison? What are those values? Maybe you'd be better off with a method called IsSpecialCustomerLocation or something that expresses the actual intent of the code.

1
  • thanks. It is fairly generic. I was just comparing an enum value - it was related to GUI programming so it didn't have any business meaning as such. Feb 11, 2009 at 9:33
1

You can also use LINQ method syntax for that task (by using System.Linq namespace):

            object[] objects = new object[10];
        objects.Contains(new MyClass());

Hmm let me think an moment... Oh you already using it. But you have put it in a separate method instead of calling it directly.

1
  • the array initializer syntax would be even cleaner... see my post. Feb 11, 2009 at 9:23
1

I would make a static class for that purpose. I don't like that solution because it adds a method to all classes which seems a bit overkill. However it does in a way go with OOD because you ask the objects to perform functions on themselves (kinda).

Still, I would go with a reusable class instead because I can see how an antipattern could form. I'm not calling it an antipattern yet, but if too many of those constructs pop up I would call that an antipattern of readability since every object would get cluttered with extension methods. I kinda look at it like namespace pollution, but class member pollution.

if (ConditionHelper.IsEqualToAny(value, value1, value2, value3)) 
{
    // Do something
}

Does the same job and doesn't pollute anything.

1
  • Coming to think of it, it suffers of a little readability problem. It has to do with that it isn't obvious that value is checked against value1, value2, value3 and so forth. value.IsEqualToAny(value1, value2, value3) does have a better readability. My concern was only about polluting members.
    – Statement
    Feb 11, 2009 at 10:10
1

Are you really intending to do Contains and guarantee you will be applying Contains on all possible objects where you might use this extension method?

If some given objects test equality by overloading operator== then your generic solution will fail. That makes it not a true equivalent of the multiple == tests. It is also a good example of the danger of writing extension methods!

The following Linq code works when you implement operator overloading as well as if you are using the default == meaning of comparing object references, to say that value is actually the same object as value1, 2, 3 or 4, given V as the object type of your values in this particular case:

V[] lv = { value, value2, value3, value4 };
if (lv.Any( v => v==value))
   // do something

Or a short-hand version:

if (new List<V>{value, value2, value3, value4 }.Any( v => v==value))
   // do something

I couldn't get the above lambda expressions to work in a generic extension method.

As a good (if irrelevant) example of what I think is a lovely, readable syntax, the idiom in Python would be

if value in (value1, value2, value3, value4):
2
  • I don't see how this is an issue. The Contains method (which is an ExtensionMethod (implemented in System.Linq.Enumerable)) uses an EqualityComparer which (in the default case) uses a == comparison. Feb 11, 2009 at 11:38
  • Maybe I implemented it wrongly or there is something about the way it is bound but when I test it the extension method fails to detect an object which is a different object but equal based on its operator== overloading.
    – Andy Dent
    Feb 11, 2009 at 12:37
0

If you are only checking an Enum value (as you said in the comment on Rogers answer), you should use a FlagsAttribute on the Enum.

[Flags]
public enum Value
{
  Value1 = 0,
  Value2 = 1,
  Value3 = 2,
  Value4 = 4
}

Value value = Value.Value1;
if (value | Value.Value1 | Value.Value2 | Value.Value3 | Value.Value4)
{
  // You can also use other bitwise operations, like & (AND), ^ (XOR) and ~ (NOT)
}

Otherwise; if it is domain specific, add it to your business logic. If it's generic, create a helper class.

1
  • I don't think that would be appropriate. Only because I want to check for a number of different values doesn't mean that the enum should be a bit field. I don't want the enum value to be a combined value. - thanks for your input though. I have been using this extension method for a while now and it has been quite useful in a number of situations. May 19, 2009 at 1:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.