Skip to main content
Bounty Ended with 250 reputation awarded by Mathieu Guindon
Typo - wasn't concentrating
Source Link
RobH
  • 17k
  • 6
  • 37
  • 72
public Entry(dynamic entity, EntityState state, MetadataContainer container)
{
    State = state;
    _entity = entity;
    _members = container.GetMembersGetConfiguredMembers();
}
public Entry(dynamic entity, EntityState state, MetadataContainer container)
{
    State = state;
    _entity = entity;
    _members = container.GetMembers();
}
public Entry(dynamic entity, EntityState state, MetadataContainer container)
{
    State = state;
    _entity = entity;
    _members = container.GetConfiguredMembers();
}
Source Link
RobH
  • 17k
  • 6
  • 37
  • 72

Just a few comments...

You've overcomplicated your Attach method:

/// <summary>
/// Registers an entity for change tracking.
/// </summary>
public void Attach<TEntity>(TEntity entity, EntityState state = EntityState.Unchanged) where TEntity : EntityBase
{
    var exists = _entries.ContainsKey(entity);
    if (exists && _entries[entity].State == state)
    {
        return;
    }
    if (exists)
    {
        _entries[entity].State = state;
        return;
    }

    _entries[entity] = new Entry(entity, state);
}

All you want to do is either add a new entry or update the current entry's state.

/// <summary>
/// Registers an entity for change tracking.
/// </summary>
public void Attach<TEntity>(TEntity entity, EntityState state = EntityState.Unchanged) where TEntity : EntityBase
{
    Entry currentEntry;
    if (_entries.TryGetValue(entity, out currentEntry)
    {
        // There's no point in checking the current vs expected state
        // as you overwrite it if it's different anyway.
        currentEntry.State = state; 
    }
    else
    {
        _entries[entity] = new Entry(entity, state);
    }
}

I'd worry about using the entity as a key in a dictionary directly as you'd be relying on unknown and unverifiable GetHashCode and Equals implementations.

Do you configure keys for your entities? If you do, I'd recommend using a string key like $"{ typeName }_{ metadata.GetKeyValue(entity) }" instead. String's GetHashCode and Equals is definitely fine :).


In general, I think you need to introduce an abstraction over the entity types mapping. At the moment, some of your methods feel a bit too low level:

public Entry(dynamic entity, EntityState state)
{
    State = state;
    _entity = entity;
    
    // hmmmm.
    _entityType = _entity.GetType();
    _members = _entityType.GetProperties(BindingFlags.Instance | BindingFlags.Public)
        .ToDictionary(property => property.Name, property => property.GetValue(entity));
}

If you introduced some MetadataContainer (which could be a property on your SageContextBase)

 public class MetadataContainer
 {
     public IDictionary<string, object> GetConfiguredMembers(Type entityType) 
     {
         // cache?
     }
 }

Then your entry constructor becomes easier to grok:

public Entry(dynamic entity, EntityState state, MetadataContainer container)
{
    State = state;
    _entity = entity;
    _members = container.GetMembers();
}