CODEWITHSUNDEEP
c#extension-methods
Patrick Klug
Patrick Klug

Reputation: 14391

Is this a good use of an ExtensionMethod?

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.

Upvotes: 10

Views: 385

Answers (8)

Ronald
Ronald

Reputation: 1815

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.

Upvotes: 0

Andy Dent
Andy Dent

Reputation: 17969

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):

Upvotes: 1

Marc Gravell
Marc Gravell

Reputation: 1062660

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

Upvotes: 5

Statement
Statement

Reputation: 4058

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.

Upvotes: 1

Alexander
Alexander

Reputation: 3744

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.

Upvotes: 1

Peter Morris
Peter Morris

Reputation: 23224

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"

Upvotes: 4

Roger Lipscombe
Roger Lipscombe

Reputation: 91825

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.

Upvotes: 1

Richard Ev
Richard Ev

Reputation: 54097

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

Upvotes: 1

Related Questions