6

I've read various questions similar to mine but none of them seem address my issue.

I've a type like this:

class MyObject<T> : IEquatable<MyObject<T>> { // no generic constraints
  private readonly string otherProp;
  private readonly T value;

  public MyObject(string otherProp, T value)
  {
    this.otherProp = otherProp;
    this.value = value;
  }

  public string OtherProp { get { return this.otherProp; } }

  public T Value { get { return this.value; } }

  // ...

  public bool Equals(MyObject<T> other)
  {
    if (other == null)
    {
       return false;
    }

    return this.OtherProp.Equals(other.OtherProp) && this.Value.Equals(other.Value);
  }
}

When T is a scalar as MyObject<int> equality works correctly, but when I defined something like MyObject<IEnumerable<int>> equality fails.

The reason is that when T is IEnumerable<T> I should call this.Value.SequenceEqual(other.Value).

Handling this difference bloats Equals(MyObject<T>) with too LOC of type check and reflection (for me, leading to a violation of SOLID/SRP).

I was not able to found this specific case in MSDN guidelines, so if anyone already faced this problem; it would be great if this knowledge can be shared.

Edit: Alternative

To K.I.S.S., I'm wondering to doing something similar:

class MyObject<T> : IEquatable<MyObject<T>> {
  private readonly IEnumerable<T> value;

  // remainder omitted
}

In this way the implementation of Equal will be a lot simple. And when I need just one value I've a collection of 1 item. Obviously T will not be an enumerable (but the data structure is private so there's no problem).

jay
  • 1,510
  • 2
  • 11
  • 19
  • If it's possible in your case, you may also add a generic constraint `where T: IEquatable`. – Guillaume Mar 19 '13 at 08:42
  • @Guillaume, I've to evaluate and reason about it... – jay Mar 19 '13 at 09:01
  • For reference I want to add the what goes in the answer under __Edit: Alternative__ was close to my real solution. I'm experiencing that there's no valid reason to not use an `IEnumerable` also when in most cases the collection of `T` will hold just one element or rather be empty. This is not the case, the collection often hold from one to few elements; so I'd say that `IEnumerable` should be _hard coded_ in the definition of `MyObject` rather than optionally passed as a whole through the type parameter (I'm talking of avoiding `MyObject>` and go for the alternative). – jay Mar 24 '13 at 19:41
  • 1
    For reference: this [extension method](https://gist.github.com/gsscoder/5271909) calls SequenceEqual with reflection. It's a draft, but in its context it actually does its job. – jay Mar 29 '13 at 16:28

4 Answers4

4

In case when T is IEnumerable<T>, your code compares two references of IEnumerable<T> and, of course, those references may be not equal. Actually, you'll get this behavior, when T will be any reference type without overridden Equals method.

If you don't want to compare references here, you should write a code, which will compare these sequences by their content. However, you should note, that sequence may be endless.

Dennis
  • 37,026
  • 10
  • 82
  • 150
  • I was about to editing the title... But now I see I correctly tagged it with `structural-equality`. – jay Mar 19 '13 at 10:09
3

You could have your MyObject<T> take an equality comparer for your type T and use that:

class MyObject<T> : IEquatable<MyObject<T>>
{
    private readonly IEqualityComparer<T> comparer;
    public MyObject(string otherProp, T value, IEqualityComparer<T> comparer)
    {
        this.comparer = comparer;
    }
    public MyObject(string otherProp, T value)
        : this(otherProp, value, EqualityComparer<T>.Default)
    {
    }

    public bool Equals(MyObject<T> other)
    {
        return OtherProp.Equals(other.OtherProp) && comparer.Equals(this.Value, other.Value);
    }
}

Then for IEnumerable<T> you can use a comparer which compares the sequences instead of the references. You might want to make use of factory methods to create your objects to ensure the same comparer type is used for the same T to ensure equality remains symmetric.

Lee
  • 142,018
  • 20
  • 234
  • 287
  • +1, well written code. Just to point it out, this would work automatically for types such as `int` (since it's mentioned in the question)? – default Mar 19 '13 at 08:42
  • @Lee +1 for factory supplied with `constructor injection`; but as stated in another comment and in my latest edit I'd like to avoid writing infrastructural code. – jay Mar 19 '13 at 10:18
  • @Lee, marked as answered because as stated in previous comment I liked the use of C.I. to pass the dependency. – jay Mar 22 '13 at 06:22
1

No reflection is needed. One option is to check if value is IEnumerable or not in your Equals method:

IEnumerable e = other.Value as IEnumerable;

if(e != null){
 // use  SequenceEqual
}else{
 // use Equals
}
alex
  • 12,464
  • 3
  • 46
  • 67
  • Have tried this solution, @alex? I think is not so simple as presented here. `SequenceEqual` needs a type parameter so you've to cast to `IEnumerable`. To say more the current instance can be a scalar or an enumerable of a different T. A lot more checks a involved and I don't think you can skip reflection at all. There's more involved: `string` should be treated as a scalar but also `string` implements IEnumerable. – jay Mar 19 '13 at 08:57
  • You can write a "universal" equality comparer method that takes two objects. If they are both enumerable, then it iterates over them. To compare items of IEnumerable it calls itself. So array of arrays would be compared correctly. You can also add some exception to string, as a performance optimization. – alex Mar 19 '13 at 09:05
  • sorry but I want keep things simple. I don't won't write and test a so general piece of infrastructural code. As you can see from my latest edit to the question, I'm looking to a K.I.S.S. solution. – jay Mar 19 '13 at 10:15
  • see my last comment to the question. Checking for `IEnumerable` is a good start, but then other stuff must be done (and it's not trivial as it could appear). – jay Mar 29 '13 at 16:35
1

this is the case where you should take help of helper class , your MyObject type may make use of individual EqualityChecker.

I would say implement a strategy pattern with static factory method will simplify your design.

something like below

class MyObject<T> : IEquatable<MyObject<T>>
{ // no generic constraints
    private readonly string otherProp;
    private readonly T value;

    public MyObject(string otherProp, T value)
    {
        this.otherProp = otherProp;
        this.value = value;
    }

    public string OtherProp { get { return this.otherProp; } }

    public T Value { get { return this.value; } }

    // ...

    public bool Equals(MyObject<T> other)
    {
        if (other == null)
        {
            return false;
        }

     var cheker =   EqualityChekerCreator<T>.CreateEqualityChecker(other.Value);

     if (cheker != null)
         return cheker.CheckEquality(this.Value, other.value);

        return this.OtherProp.Equals(other.OtherProp) && this.Value.Equals(other.Value);
    }
}

public static class  EqualityChekerCreator<T>
{
    private static IEqualityCheker<T> checker;

    public static IEqualityCheker<T>  CreateEqualityChecker(T type)
    {
        var currenttype = type as IEnumerable<T>;

        if(currenttype!=null)
            checker = new SequenceEqualityChecker<T>();

        return checker;
    }
}

public  interface  IEqualityCheker<in T>
{
    bool CheckEquality(T t1, T t2);
}

public class SequenceEqualityChecker <T> : IEqualityCheker<T>
{
    #region Implementation of IEqualityCheker<in T>

    public bool CheckEquality(T t1, T t2)
    {
        // implement method here;
    }

    #endregion
}
TalentTuner
  • 17,262
  • 5
  • 38
  • 63