Tracking Entity Changes (not EF)

So, I kept refactoring my Sage300 API wrapper - I wanted the client code to feel just like using Entity Framework - this is the closest I got to it:

using (var context = new SageContext(/*redacted credentials*/))
{
    context.Open();

    var header = context.PurchaseOrderHeaders.Single(po => po.Number == "NETAPI99");

    header.Description, "update test";
    var detail = header.Details.First();
    detail.QuantityOrdered = 42;
    context.SaveChanges();
}

The above selects a specific PurchaseOrderHeader entity, changes its Description to "update test", then selects the first PurchaseOrderDetail child entity and sets its QuantityOrdered to 42... and then sends the changes over to the Sage300 API.

How did this become possible? With quite a bit of code. I implemented a very basic change tracker - the first thing I needed was an EntityState:

public enum EntityState
{
    Untracked,
    Unchanged,
    Modified,
    Added,
    Deleted
}

I needed a way to somehow support navigation properties. For about a split second I thought of generating proxy types at runtime.. and then decided to keep calm and use a base class instead:

```
public abstract class EntityBase
{
protected EntityBase()
{
InitializeNavigationChildProperties();
}

// ReSharper disable once CollectionNeverQueried.Local -- values acquired via reflection
private readonly IDictionary _navigationProperties = new Dictionary();
private void InitializeNavigationChildProperties()
{
_navigationProperties.Clear();

var properties = from property in GetType().GetProperties()
where property.GetMethod != null && property.GetMethod.IsVirtual
&& property.PropertyType.IsGenericType
&& property.PropertyType.IsInterface
select property;

foreach (var property in properties)
{
var entityType = property.Propert

if (lastResult == null)
                {
                    throw new InvalidOperationException("Sequence contains no element matching specified criteria.");
                }
                return lastResult;

For this sort of stuff, I'd write a throwIfNull wrapper. You have many of these null checks and most of them are on the wrong abstraction level, I think. Especially in Execute, what you really want to do in a big method like that is program on a higher abstraction level where all you're doing is applying operations, not having to filter various results.

You also have this:

if (constantExpression != null)
    {
        if (constantExpression.Value is ViewSet)
        {
            return viewSet.Select(string.Empty);
        }
    }

Which is a nested if statement with no elses anywhere, looks like edit scarring to me.

/// 
/// Commits all changes to the underlying Sage views.
/// 
public void SaveChanges()
{
    var deleted = _tracker.TrackedEntities(EntityState.Deleted).ToList();
    foreach (var entity in deleted)
    {
        Delete(entity);
    }

    var inserted = _tracker.TrackedEntities(EntityState.Added).ToList();
    foreach (var entity in inserted)
    {
        Insert(entity);
    }

    _tracker.VerifyModifiedState();
    var updated = _tracker.TrackedEntities(EntityState.Modified).ToList();
    foreach (var entity in updated)
    {
        Update(entity);
    }

    _tracker.AcceptChanges();
}

Again, go for the higher level view if possible.

I'd prefer to read this:

/// 
/// Commits all changes to the underlying Sage views.
/// 
public void SaveChanges()
{
    DeleteAllOf(_tracker.TrackedEntities(EntityState.Deleted).ToList());
    InsertAllOf(_tracker.TrackedEntities(EntityState.Added).ToList());

    _tracker.VerifyModifiedState();

    UpdateAllOf(_tracker.TrackedEntities(EntityState.Modified).ToList());

    _tracker.AcceptChanges();
}

I'd even go one step further:

private List getTrackedEntitiesInState(EntityState state) where TEntity : EntityBase
{
    return _tracker.TrackedEntities(state).ToList();
}

/// 
/// Commits all changes to the underlying Sage views.
/// 
public void SaveChanges()
{
    DeleteAllOf(getTrackedEntitiesInState(EntityState.Deleted));
    InsertAllOf(getTrackedEntitiesInState(EntityState.Added));

    _tracker.VerifyModifiedState();

    UpdateAllOf(getTrackedEntitiesInState(EntityState.Modified));

    _tracker.AcceptChanges();
}

Maybe even use ICollection instead of List as expected types.

This code in GetNavigationChildEntities...

if (readFromViewSet)
    {
        // reading record from database; hydrate navigation properties by sending a SELECT query to the server.
        var constructedType = typeof(ViewSet<>).MakeGenericType(typeof(TChildEntity));
        dynamic viewSet = Convert.ChangeType(childViewSet, constructedType);
        WriteKeys(entity);
        foreach (var childEntity in viewSet.Select(string.Empty))
        {
            result.Add(childEntity);
        }

        return result;
    }

It's the result of using a flag argument.

Maybe you can't get around the use of a flag argument. Maybe you can. In the situations where you can't, try to make the flag argument a clear point of separation. In this case, make a separate function so that the existence of the flag argument is easier to spot. Maybe in later refactorings you can spot some cases where you don't need the flag argument and can directly call the function you would have acccessed via the flag argument.

Also, if you can somehow iterate over viewSet.Select(string.Empty), is there anything stopping you from using result.AddRange(viewSet.Select(string.Empty))?

/// 
/// Inserts a single new record into a flat view, or of a detail record in a composed header/detail view.
/// 
/// The entity that contains the key field and values to insert.
private void Insert(TEntity entity)
    where TEntity : EntityBase
{
    BeginInsert(entity);
    InsertChildEntities(entity);
    FinalizeInsert();
}

This I like. Higher order functions. Start inserting the entity, insert the child entities, and then finish up.

Scrolling down, it's good that you did, because those three functions look pretty big.

StackExchange Code Review Q#121254, answer score: 2