11
\$\begingroup\$

Recently, I wanted to see how I might could track state changes to objects, and manage relationships (1 to 1, 1 to N, N to N) between types in C#. This was a really interesting project, and I'm wondering how it might be improved.

Full code

The full code can be found here, which includes a test project.

Basics: TrackableEntity, EntityManager and TrackableProperty

Everything in my EntityTracker project works with TrackableEntity, which does what you might think.

/// <summary>Base type that supports change tracking </summary>
public abstract class TrackableEntity
{
    public int Id
    {
        get; 
        internal set;
    }

    public bool IsDirty
    {
        get;
        internal set; 
    }

    public void Commit()
    {
        IsDirty = false;
    }

    public TrackableEntity()
    {
        EntityManager.Instance.Create(this);
    }

    public void Delete()
    {
        EntityManager.Instance.Delete(this);
    }
}

All the TrackableEntity objects are stored in a globally accessible repository, EntityManager. I'll save discussion of the Delete method referenced by TrackableEntity for later.

/// <summary>
/// Container class to hold TrackableEntity objects
/// </summary>
public class EntityManager
{
    private static EntityManager instance = new EntityManager();
    private int next;
    private Dictionary<int, TrackableEntity> entities;

    private EntityManager()
    {
        entities = new Dictionary<int, TrackableEntity>();
    }

    public static EntityManager Instance
    {
        get { return instance; }
    }

    /// <summary>Adds the TrackableEntity in the container</summary>
    public void Create(TrackableEntity entity)
    {
        entity.Id = next++;
        entities[entity.Id] = entity;
    }

    /// <summary>Gets the TrackableEntity stored at id</summary>
    public TrackableEntity Lookup(int id)
    {
        return entities.ContainsKey(id) ? entities[id] : null;
    }
}

To actually track changes to properties in TrackableEntity, I created an object to wrap each property which allows me to manage the state of the IsDirty flag. This object actually holds all values for a type that declares a TrackableProperty and looks up the owner's value by id.

/// <summary>Wrapper property to track changes</summary>
public class TrackableProperty<T> where T : IEquatable<T>
{
    Dictionary<int, T> values = new Dictionary<int, T>();

    /// <summary>Gets the owner's value</summary>
    public T GetValue(TrackableEntity owner)
    {
        return values.ContainsKey(owner.Id) ?
            values[owner.Id] : default(T);
    }

    /// <summary>Sets the owner's value</summary>
    public void SetValue(TrackableEntity owner, T value)
    {
        if (values.ContainsKey(owner.Id) && values[owner.Id].Equals(value))
            return;
        owner.IsDirty = true; values[owner.Id] = value;
    }
}

Here's a simple example of how TrackableEntity and TrackableProperty are used:

[DebuggerDisplay("Name = {Name}")]
abstract class NamedEntity : RelationshipEntity
{
    public static readonly TrackableProperty<string> NameProperty
        = new TrackableProperty<string>();

    public string Name
    {
        get { return NameProperty.GetValue(this); }
        set { NameProperty.SetValue(this, value); }
    }
}

Relationships

The three relationships (1 to 1, 1 to N, and N to N) I support all implement the following generic interface.

/// <summary>Base type for relationships between RelationshipEntity objects</summary>
public interface IRelationship<T1, T2>
    where T1 : RelationshipEntity
    where T2 : RelationshipEntity
{
    void CreateRelationship(T1 t1, T2 t2);
    void DeleteRelationship(RelationshipEntity entity);
}

/// <summary>Base type for relationships between TrackableEntity objects </summary>
public abstract class RelationshipEntity : TrackableEntity
{
    public abstract void Accept(IRelationshipVisitor visitor);
}

Implementation of Relationship1To1

The efficiency of navigating in the reverse direction (i.e. getting T1 from T2) could be improved. For now, however, I'm just using a linear search to make sure the relationship is maintained.

    /// <summary>Represents a 1 to 1 relationship</summary>
public class Relationship1To1<T1, T2> : IRelationship<T1, T2>
    where T1 : RelationshipEntity
    where T2 : RelationshipEntity
{
    Dictionary<int, int> forward = new Dictionary<int, int>();

    /// <summary>
    /// Creates the relationship; t1 and t2
    /// may only reference one another
    /// </summary>
    public void CreateRelationship(T1 t1, T2 t2)
    {
        DeleteRelationship(t1);
        DeleteRelationship(t2);
        forward[t1.Id] = t2.Id;
    }

    public void DeleteRelationship(RelationshipEntity entity)
    {
        var kvps = forward.Where(kvp =>
            kvp.Key == entity.Id || kvp.Value == entity.Id);

        foreach (var item in kvps.ToList())
            forward.Remove(item.Key);
    }

    public T2 GetForward(T1 owner)
    {
        return forward.ContainsKey(owner.Id) ?
            (T2)EntityManager.Instance.Lookup(forward[owner.Id]) : null;
    }

    public T1 GetReverse(T2 owner)
    {
        foreach (KeyValuePair<int, int> kvp in forward)
            if (kvp.Value == owner.Id)
                return (T1)EntityManager.Instance.Lookup(kvp.Key);
        return null;
    }
}

Implementation of Relationship1ToN

This seemed pretty straightforward. I use a dictionary to ensure that each child has only one parent.

/// <summary>Represents a 1 to N relationship</summary>
public class Relationship1ToN<T1, T2> : IRelationship<T1, T2>
    where T1 : RelationshipEntity
    where T2 : RelationshipEntity
{
    Dictionary<int, T1> children = new Dictionary<int, T1>();

    /// <summary>
    /// Creates the relationship; t2 may only have one owner, t1
    /// </summary>
    public void CreateRelationship(T1 t1, T2 t2)
    {
        children[t2.Id] = t1;
    }

    public void DeleteRelationship(RelationshipEntity entity)
    {
        var kvps = children.Where(x => x.Key == entity.Id || x.Value.Id == entity.Id);

        foreach (var item in kvps.ToList())
            children.Remove(item.Key);
    }

    public IEnumerable<T2> GetFoward(T1 parent)
    {
        return children.Where(pair => pair.Value.Id == parent.Id)
            .Select(pair => (T2)EntityManager.Instance.Lookup(pair.Key));
    }

    public T1 GetReverse(T2 owner)
    {
        return children.ContainsKey(owner.Id) ?
            children[owner.Id] : null;
    }
}

Implementation of RelationshipNToN

This seemed most straightforward. Anyone can own anyone:

/// <summary>Represents an N to N relationship</summary>
public class RelationshipNToN<T1, T2> : IRelationship<T1, T2>
    where T1 : RelationshipEntity
    where T2 : RelationshipEntity
{
    Dictionary<int, HashSet<int>> forward = new Dictionary<int, HashSet<int>>();
    Dictionary<int, HashSet<int>> reverse = new Dictionary<int, HashSet<int>>();

    /// <summary>Creates the relationship</summary>
    public void CreateRelationship(T1 t1, T2 t2)
    {
        if (!forward.ContainsKey(t1.Id)) forward[t1.Id] = new HashSet<int>();
        if (!reverse.ContainsKey(t2.Id)) reverse[t2.Id] = new HashSet<int>();

        forward[t1.Id].Add(t2.Id);
        reverse[t2.Id].Add(t1.Id);
    }

    public void DeleteRelationship(RelationshipEntity entity)
    {
        var kvps = forward.Where(x => x.Key == entity.Id || x.Value.Contains(entity.Id))
            .Union(reverse.Where(x => x.Key == entity.Id || x.Value.Contains(entity.Id)));

        foreach (var kvp in kvps.ToList())
        {
            reverse.Remove(kvp.Key);
            forward.Remove(kvp.Key);
        }
    }

    public IEnumerable<T2> GetForward(T1 owner)
    {
        return forward.ContainsKey(owner.Id) ?
            from key in forward[owner.Id] 
            select (T2)EntityManager.Instance.Lookup(key) : Enumerable.Empty<T2>();
    }

    public IEnumerable<T1> GetReverse(T2 owner)
    {
        return reverse.ContainsKey(owner.Id) ?
            from key in reverse[owner.Id] 
            select (T1)EntityManager.Instance.Lookup(key) : Enumerable.Empty<T1>();
    }
}

Deleting relationships

If we delete an TrackableEntity, we need to remove all references to it. To me, it seemed that the best way to visit all relationships in the object graph was with the visitor pattern. At each relationship, we call implementation of DeleteRelationship for the entity to be deleted.

RelationshipEntity must be able to accept an IRelationshipVisitor. Since we have three implementations of IRelationship, we should provide three ways to visit.

/// <summary>Base type for visiting relationships in the object graph</summary>
public interface IRelationshipVisitor
{
    void Visit<T1, T2>(Relationship1To1<T1, T2> relationship)
        where T1 : RelationshipEntity
        where T2 : RelationshipEntity;

    void Visit<T1, T2>(Relationship1ToN<T1, T2> relationship)
        where T1 : RelationshipEntity
        where T2 : RelationshipEntity;

    void Visit<T1, T2>(RelationshipNToN<T1, T2> relationship)
        where T1 : RelationshipEntity
        where T2 : RelationshipEntity;
}

The visitor for deletion is RelationshipBreaker (omitted here for brevity). To delete in the EntityManager, all we have to do is visit the relationships and then delete the entity.

    /// <summary>
    /// Deletes the TrackableEntity from the container
    /// and removes all references from relationships 
    /// </summary>
    public void Delete(TrackableEntity entity) 
    {
        if (entity is RelationshipEntity)
        {
            var casted = (RelationshipEntity)entity;
            casted.Accept(new RelationshipBreaker(casted));
        }
        entities.Remove(entity.Id);
    }

Example code

Here are some examples of entities with relationships

class A : NamedEntity
{
    public static readonly Relationship1To1<A, B> relationship
        = new Relationship1To1<A, B>();

    public B B
    {
        get { return relationship.GetForward(this); }
        set { relationship.CreateRelationship(this, value); }
    }

    public C Parent
    {
        get { return C.relationship.GetReverse(this); }
    }

    public override void Accept(IRelationshipVisitor visitor)
    {
        visitor.Visit<A, B>(A.relationship);
        visitor.Visit<C, A>(C.relationship);
    }
}

class B : NamedEntity
{
    public A A
    {
        get { return A.relationship.GetReverse(this); }
        set { A.relationship.CreateRelationship(value, this); }
    }

    public override void Accept(IRelationshipVisitor visitor)
    {
        visitor.Visit<A, B>(A.relationship);
    }
}

class C : NamedEntity
{
    public static readonly Relationship1ToN<C, A> relationship
        = new Relationship1ToN<C, A>();

    public IEnumerable<A> Children
    {
        get { return relationship.GetFoward(this); }
    }

    // how to avoid this -- would like for the relationship to return
    // a custom collection (this has add/remove etc) that enforces the relationship
    public void Add(A item)
    {
        relationship.CreateRelationship(this, item);
    }

    public override void Accept(IRelationshipVisitor visitor)
    {
        visitor.Visit<C, A>(relationship);
    }
}

Questions and points to review

  • In the implementation of Delete, I cast to a relationship type. I had tried providing two implementations of Delete; one for the TrackableEntity (no relationships) and another for RelationshipEntity. This didn't work, however (the delete call for a test entity kept going through the TrackableEntity overload and not to the RelationshipEntity overload). Why is this?
  • I expose the children of a Relationship1ToN as an IEnumerable<T2>. As a result, I have to provide additional methods to add an item to this relationship that are exposed in the entity classes. Is there any way to abstract this? Maybe instead of returning an IEnumerable<T2>, would it be possible to return an ICollection<T2>, and provide checks to maintain the relationship?
  • General design -- how did I do?
\$\endgroup\$
  • 2
    \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Heslacher Sep 18 '15 at 13:32
  • \$\begingroup\$ My mistake -- I had thought putting code in a separate section (indeed, not in any of the code I've asked be reviewed) might be okay. \$\endgroup\$ – rookie Sep 18 '15 at 17:10
  • \$\begingroup\$ I always find tracking systems too heavy, and yet not rich and flexible enough. There is a reason Microsoft has given up on SelfTrackingEntities. Your attempt is very nice one. It does not smell like boiler-plate code. Well done. \$\endgroup\$ – dfhwze May 25 at 21:11
7
+200
\$\begingroup\$

General

If you need to get a value of a Dictionary<TKey, TValue> you shouldn't us ContainsKey() together with the Item property getter but TryGetValue(), because by using ContainsKey() in combination with the Item getter you are doing the check if the key exists twice.

See also: what-is-more-efficient-dictionary-trygetvalue-or-containskeyitem

TrackableEntity

This abstract class seems almost fine to me. The only missing is the lack of documentation. A framework should always have enough documentation targeting the the parts which are accesible from outside. So each public and protected property/method should be documentated.

While we talk about scope, an abstract class only serves one purpose, it is intended to be implemented. By making a setter of a property internal one who wants to implement such a class can't set these properties (if it is allowed to set them).

If I would implement the abstract TrackableEntity class, I would maybe want to set the IsDirty property to a specific value, but because its internal I just can't. So making properties which you feel should be set from outside protected would be a good decision.


EntityManager singleton

Hmmm, a singletone is coming along. How am I be able to write unit tests for a singleton or for classes which uses this singleton? Hmmm, I just can't easily do this, because I can't reset the state of this singleton between tests which is bad.

For instance assume we want to test the creation of a TrackableEntity object and because it comes handy we assert that TrackableEntity.Id == 1 so, no problem there, the test passes.

Next we write another test which test for TrackableEntity.Delete() and we assert with EntityManager.Instance.Lookup(TrackableEntity.Id) == null).

Then you run both tests and they passed and you ask yourself hey what is this guy talking about but then you change the order of the tests and hence the first written test fails because after the test for Delete the private int next of the EntityManager is 1. So if the creation test is executed the assert will fail.

What are possible ways out of this ? You can let EntityManager implement an interface IEntityManager having Create(), Lookup()andDelete()methods and use a private field inTrackableEntitywhich is holding a reference ofIEntityManager` which is constructor injected.

The changes you would need to do are simple, some thing along these lines

public interface IEntityManager
{
    TrackableEntity Lookup(int id);
    void Create(TrackableEntity entity);

    void Delete(TrackableEntity entity);
}
public class EntityManager : IEntityManager
{
    private static IEntityManager instance = new EntityManager();
    private int next;
    private Dictionary<int, TrackableEntity> entities;

    private EntityManager()
    {
        entities = new Dictionary<int, TrackableEntity>();
    }

    public static IEntityManager Instance
    {
        get { return instance; }
    }

    /// <summary>Adds the TrackableEntity in the container</summary>
    public void Create(TrackableEntity entity)
    {
        entity.Id = next++;
        entities[entity.Id] = entity;
    }

    /// <summary>Gets the TrackableEntity stored at id</summary>
    public TrackableEntity Lookup(int id)
    {
        TrackableEntity entity;
        entities.TryGetValue(id, out entity);

        return entity;
    }

    public void Delete(TrackableEntity entity)
    {
        // we will target this later
        throw new NotImplementedException();
    }
}  
public abstract class TrackableEntity
{
    private readonly IEntityManager manager;
    public TrackableEntity()
    {
        manager = EntityManager.Instance;
        manager.Create(this);
    }
    public TrackableEntity(IEntityManager manager)
    {
        this.manager = manager;
        manager.Create(this);
    }
    public int Id
    {
        get;
        internal set;
    }

    public bool IsDirty
    {
        get;
        internal set;
    }

    public void Commit()
    {
        IsDirty = false;
    }

    public void Delete()
    {
        manager.Delete(this);
    }
}

Now you use a mocked IEntityManager obejct for unit tests.

Another maybe easier aproach would be like described in the answer to unit-testing-singletons

Short version: do not write your singletons as singletons. Write them as normal classes, and call them via an Inversion of Control container, where you have configured the class to be a singleton instead.


TrackableProperty

Using multiple code statements in the same line is hard to read and to debug and should be avoided.

Access to a dictionary should be changed like written above like so

/// <summary>Wrapper property to track changes</summary>
public class TrackableProperty<T> where T : IEquatable<T>
{
    Dictionary<int, T> values = new Dictionary<int, T>();

    /// <summary>Gets the owner's value</summary>
    public T GetValue(TrackableEntity owner)
    {
        T foundValue;
        values.TryGetValue(owner.Id, out foundValue);
        return foundValue;
    }

    /// <summary>Sets the owner's value</summary>
    public void SetValue(TrackableEntity owner, T value)
    {
        T foundValue;
        if (values.TryGetValue(owner.Id, out foundValue) && foundValue.Equals(value)
        {
            return;
        }
        owner.IsDirty = true;  
        values[owner.Id] = value;
    }
}  

Relationship1To1

  • I would make it more clear who the owner of the object is by changing T1 to TOwner.

  • Changes to dictionary access should be made like mentioned.


Relationship1ToN

  • Change T1 to TParent to make it more obvious who the parent is.
  • Changes to dictionary access should be made like mentioned.

RelationshipNToN

  • Changes to dictionary access should be made like mentioned.

EntityManager.Delete()

In the implementation of Delete, I cast to a relationship type. I had tried providing two implementations of Delete; one for the TrackableEntity (no relationships) and another for RelationshipEntity. This didn't work, however (the delete call for a test entity kept going through the TrackableEntity overload and not to the RelationshipEntity overload). Why is this?

public void Delete(TrackableEntity entity) 
{
    if (entity is RelationshipEntity)
    {
        var casted = (RelationshipEntity)entity;
        casted.Accept(new RelationshipBreaker(casted));
    }
    entities.Remove(entity.Id);
}  

To eplain this I will use a simple example like so

public abstract class TheAbstractType { }
public class TheInheritingType : TheAbstractType { }

class Program
{
    static void Main(string[] args)
    {
        Program p = new Program();

        TheInheritingType a = new TheInheritingType ();
        TheAbstractType b = new TheInheritingType ();

        p.Delete(a);
        p.Delete(b);
    }
    public void Delete(TheAbstractType item)
    {
        // first
    }
    public void Delete(TheInheritingType item)
    {
        // second
    }
}  

Calling p.Delete(a); will result in the execution of the second Delete() because we have declared a as TheInheritingType.

Calling p.Delete(b); will result in the execution of the first Delete() because we have declared b as TheAbstractType.


I expose the children of a Relationship1ToN as an IEnumerable<T2>. As a result, I have to provide additional methods to add an item to this relationship that are exposed in the entity classes. Is there any way to abstract this? Maybe instead of returning an IEnumerable<T2>, would it be possible to return an ICollection<T2>, and provide checks to maintain the relationship?

You will be in the same trouble if you are returning an ICollection<T2> instead of an IEnumerable<T2> because both are only a part of the whole collection (aka Dictionary) of the Relationship1ToN.

If you for instance add an Entity to this ICollection<T2> this won't be reflected in the Relationship1ToN.

I would add the relationships as properties to the RelationshipEntity's and would initialize them only when first accessed.

After thinking about it, this idea isn't the brightest one because it breaks the encapsulation of the class. A entity could just mess with the provided relationships of a different entity. IMHO the best way would be to just add the maintaining methods to the RelationshipEntity class and keep the relationships as private fields in the class (initializing if accessed too).

If you have the feeling that this would add to much memory overhead you can add a mechanism to let empty relationships expire.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer. I'm not that concerned about performance, here. As a matter of opinion, would you agree that ContainsKey is "good enough" and more readable? Others have remarked that the performance difference is negligible. Also, is there any chance you can offer any advice on one of the points I've asked get reviewed? \$\endgroup\$ – rookie Sep 16 '15 at 15:17
  • 1
    \$\begingroup\$ Its not about the performnce difference between TryGetValue and ContainsKey but between TryGetValue and ContainsKey() together with accessing the Item's getter. \$\endgroup\$ – Heslacher Sep 16 '15 at 15:20
  • \$\begingroup\$ Thank you for your thoughtful answer. I hesitate to mark the IsDirty property protected because that would mean that TrackableProperty can't set it (I want the system to track changes, and not have the entities themselves tracking the changes). I can fix the EntityManager.Delete() problem by adding a Delete method to RelationshipEntity (hide the method). This seems less than ideal -- is there a better way? \$\endgroup\$ – rookie Sep 18 '15 at 12:54
  • \$\begingroup\$ If you're able to review the second point in my "Questions and points to review" section, I will happily reward you the full bounty. \$\endgroup\$ – rookie Sep 19 '15 at 17:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.