CODEWITHSUNDEEP
c#.netlinqlinq-to-sqlentityset
Marc Gravell
Marc Gravell

Reputation: 1062975

EntitySet - is there a sane reason that IList.Add doesn't set assigned?

There are 3 ways of adding items to most lists...

and you normally expect them to behave more or less the same. However, LINQ's EntitySet<T> is... peculiar on both 3.5 and 4.0; the IList API does not flag the set as "assigned" - the other two mechanisms do - this sounds trivial, but it is important in that it heavily influences serialization (i.e. causes it to be skipped) in the boilerplate code.

Example:

EntitySet<string> set1 = new EntitySet<string>();
set1.Add("abc");
Debug.Assert(set1.Count == 1); // pass
Debug.Assert(set1.HasLoadedOrAssignedValues, "direct"); // pass

EntitySet<string> set2 = new EntitySet<string>();
IList<string> typedList = set2;
typedList.Add("abc");
Debug.Assert(set2.Count == 1); // pass
Debug.Assert(set2.HasLoadedOrAssignedValues, "typed list"); // pass

EntitySet<string> set3 = new EntitySet<string>();
IList untypedList = set3;
untypedList.Add("abc");
Debug.Assert(set3.Count == 1); // pass
Debug.Assert(set3.HasLoadedOrAssignedValues, "untyped list"); // FAIL

Now... this is deeply surprising to me; so much so that it took me over 2 hours of tracking upwards through code to isolate what was happening. So...

is there any sane reason for this? Or is this just a bug?

(FWIW, there was also an issue in set.Assign(set) in 3.5, but this is now fixed in 4.0.)

Upvotes: 40

Views: 3323

Answers (3)

Ethan Cabiac
Ethan Cabiac

Reputation: 4993

Interestingly, this has been identified for several versions now (you stated that a 3.5 issue was fixed in 4.0). Here is a post from 2007. The rest of the IList methods in 4.0 are correctly tied to the IList<T> methods. I think that there are 2 likely explanations (of the bug/feature variety):

  1. This is an actual bug that Microsoft has not yet fixed.
  2. This is a feature that some other Microsoft code is exploiting leveraging to add items without setting the HasLoadedOrAssignedValues.

It is probably both - a bug that other code inside the framework is counting on. Sounds like someone said to themselves:

No one is really going to cast this into an IList and then call the Add method, right?

Upvotes: 20

Rick Sladkey
Rick Sladkey

Reputation: 34250

Surprisingly, the difference seems rooted in the fact that the IList.Add and IList<T>.Add methods actually have different semantics:

  • The IList.Add method fails if the entity being added is already present
  • The LIst<T>.Add method removes and then re-adds an entity if is is already present

The apparent reason for this difference is that IList.Add interface method is defined to return the index of the added entity, which for a typical implementation of IList.Add will always be the Count of the collection prior to the Add.

In any case, because the two implementations are intentionally different, it seems the authors simply accidentally omitted the this.OnModified() call in the IList.Add version.

Upvotes: 8

StriplingWarrior
StriplingWarrior

Reputation: 156544

Looks like a bug to me. ILSpy shows the differences between the two implementations:

int IList.Add(object value)
{
    TEntity tEntity = value as TEntity;
    if (tEntity == null || this.IndexOf(tEntity) >= 0)
    {
        throw Error.ArgumentOutOfRange("value");
    }
    this.CheckModify();
    int count = this.entities.Count;
    this.entities.Add(tEntity);
    this.OnAdd(tEntity);
    return count;
}

// System.Data.Linq.EntitySet<TEntity>
/// <summary>Adds an entity.</summary>
/// <param name="entity">The entity to add.</param>
public void Add(TEntity entity)
{
    if (entity == null)
    {
        throw Error.ArgumentNull("entity");
    }
    if (entity != this.onAddEntity)
    {
        this.CheckModify();
        if (!this.entities.Contains(entity))
        {
            this.OnAdd(entity);
            if (this.HasSource)
            {
                this.removedEntities.Remove(entity);
            }
            this.entities.Add(entity);
            this.OnListChanged(ListChangedType.ItemAdded, this.entities.IndexOf(entity));
        }
        this.OnModified();
    }
}

It looks like the IList implementation simply neglects to call a couple of event invokers (OnListChanged and OnModified) that LINQ to SQL probably relies on to track its changes. If this had been intentional, I would have expected them to also leave out the call to OnAdd.

Why they don't simply have IList.Add cast the value to TEntity and call the generic Add method is beyond me.

Upvotes: 4

Related Questions