4

I have the code below which implements a logic I need at two places in my code. The code is written in generic way.

It has a defect in it. It does not check whether the given object already exist in the collectionToBeExtended or not. It just add to it, and I will have twice the same item.

I use this code managing two collections, one contains Tags, the other one contains Link entities (POCO). They do not have the same parent object, nor interface. For managing the situation below I don't want to introduce a new common interface for them.

In order to fix the defect above I need put custom logic within the foreach to check whether the given object already member of the collection or not. The Tag and Entity object are different, so there is common ground for comparison. You can see the Tag implementation below.

DRY principle says clearly I should not create the exact implementations, instead I should create some generic solution can deal with both case, or a third and fourth one. My first thought was that, I introduce a switch where I can place custom logic based on object type, but Martin Fowler says in his Refactoring book that switch-cases are smelly areas.

My current knowledge and experience is not enough to solve this issue, so I really appreciate any help!

Generic implementation:

private ICollection<T> AddTo<T> ( IEnumerable<T> toBeAdded , ICollection<T> collectionToBeExtended )
    {

        if ( collectionToBeExtended == null )
        {
            collectionToBeExtended = new List<T> ( );
        }

        if ( toBeAdded != null )
        {
            foreach ( T obj in toBeAdded )
            {
                    collectionToBeExtended.Add(obj);
            }
        }

        return collectionToBeExtended;
    }

Tag implementation:

private ICollection<Tag> AddTo ( IEnumerable<Tag> toBeAdded , ICollection<Tag> collectionToBeExtended )
    {

        if ( collectionToBeExtended == null )
        {
            collectionToBeExtended = new List<Tag> ( );
        }

        if ( toBeAdded != null )
        {
            foreach ( Tag tag in toBeAdded )
            {
                if (!collectionToBeExtended.Any(c => c.Id == tag.Id && c.Name == tag.Name))
                {
                    collectionToBeExtended.Add(tag);
                }
            }
        }

        return collectionToBeExtended;
    }
AndrasCsanyi
  • 3,943
  • 8
  • 45
  • 77

4 Answers4

3

All types in C# can equated with x.Equals(y). You can override this equality if you desire.

I would just use IEnumerable<A> to solve this problem. If you want an ICollection<A> afterwards then you can construct a collection from the IEnumerable<A>.

collectionToBeExtended.Concat(toBeAdded).Distinct()
erisco
  • 14,154
  • 2
  • 40
  • 45
  • +1 I think this is best. Note that the result will also remove any duplicates in the original `collectionToBeExtended`, which may or may not be expected. (Seems to be implied by the question.) – Will Ray Apr 30 '16 at 17:27
2

I hope this isn't overkill, but extension methods seem to do the trick for me. I hope it helps you too.

public static ICollection<X> AddIf<X>(this ICollection<X> myarray, X val, Func<X, X, bool> check)
    {
        if (myarray.All<X>((c) => check(c, val))) myarray.Add(val);
        return myarray;
    }

This can then be used as

List<string> test = new List<string> { "1", "2", "3", "4", "5" };
test.AddIf<string>("1", (a, b) => !a.Equals(b)); //this will not be added
test.AddIf<string>("6", (a, b) => !a.Equals(b)); //this will be added

Or in your case,

collectionToBeExtended.AddIf((c, tag) => !c.Id.Equals(tag.Id) && !c.Name.Equals(tag.Name))

Make sure to put the extension method in a static class though.

Ikechi Michael
  • 489
  • 3
  • 6
2

You can implement IEquatable<T> for Tag

public class Tag : IEquatable<Tag>
{
    public int Id { get; set; }
    public string Name { get; set; }
    public bool Equals(Tag other)
    {
        return Id == other.Id && Name == other.Name;
    }

    ...
}

Then your codes would be

private ICollection<T> AddTo ( IEnumerable<T> toBeAdded , ICollection<T> collectionToBeExtended )
{

    if ( collectionToBeExtended == null )
    {
        collectionToBeExtended = new List<T> ( );
    }

    if ( toBeAdded != null )
    {
        foreach ( T t in toBeAdded )
        {
            if (!collectionToBeExtended.Contains(t))
            {
                collectionToBeExtended.Add(t);
            }
        }
    }

    return collectionToBeExtended;
}

You have to implement IEquatable<T> for all the classes you want to pass them to the AddTo method. And remember to implement it correctly. See Is there a complete IEquatable implementation reference?.

There is another option. Just add another parameter to compare the two entities, like

private ICollection<T> AddTo<T>(IEnumerable<T> toBeAdded, ICollection<T> collectionToBeExtended, Func<T, T, bool> comparer)
{

    if (collectionToBeExtended == null)
    {
        collectionToBeExtended = new List<T>();
    }

    if (toBeAdded != null)
    {
        foreach (T t in toBeAdded)
        {
            if (!collectionToBeExtended.Any(existingT => comparer(t, existingT))))
            {
                collectionToBeExtended.Add(t);
            }
        }
    }

    return collectionToBeExtended;
}

Then you call this method like AddTo(tags, collection, (t1, t2) => t1.Id == t2.Id && t1.Name == t2.Name);. If either of the collection has large data, the performance should be considered.

Community
  • 1
  • 1
Kirin Yao
  • 1,606
  • 2
  • 14
  • 21
1

If you implemented IEquatable<T> in Tag and any other classes you use with this method, and add a generic constraint where T:IEquatable<T> to your method you can now scan the collection for existing members and compare them for equality.

But you'd simply be duplicating code that already exists in HashSet<T> so why not use that class with the constructor HashSet<T> Constructor (IEqualityComparer<T>)?

Ian Mercer
  • 38,490
  • 8
  • 97
  • 133