Simple change tracker for POCOs
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
5
down vote
favorite
Problem
I have a list of POCO model objects (approximatly 1.000 to 10.000) and I want to track changes on them:
- Check if at least one of the items has changed (to show the user that something changed)
- Get all new, deleted and modified objects to update the state in the database
For the first version, it is enough to support POCOs with primitive property types. However, probably I'll extend it in future to support complex property types.
Solution
public class ChangeTracker<TEntity> where TEntity : class
private static readonly PropertyInfo thePropertyInfos;
private readonly List<TEntity> myEntities = new List<TEntity>();
private readonly List<TEntity> myDeletedEntites = new List<TEntity>();
private readonly HashSet<TEntity> myNewEntities = new HashSet<TEntity>();
private readonly Dictionary<TEntity, object> myStates = new Dictionary<TEntity, object>();
static ChangeTracker()
thePropertyInfos = typeof(TEntity).GetProperties().ToArray();
public void Add(TEntity entity)
if (myStates.ContainsKey(entity))
throw new InvalidOperationException("It is not possible to add an entity twice.");
myStates.Add(entity, new object[thePropertyInfos.Length]);
myNewEntities.Add(entity);
myEntities.Add(entity);
public void Delete(TEntity entity)
myEntities.Remove(entity);
if (!myNewEntities.Remove(entity))
myDeletedEntites.Add(entity);
public void AcceptChanges()
myNewEntities.Clear();
myDeletedEntites.Clear();
foreach (var entity in myEntities)
myStates[entity] = GetState(entity);
public bool HasChanges()
EntityStates.Modified
private static object GetState(TEntity entity)
return thePropertyInfos
.Select(pi => pi.GetValue(entity))
.ToArray();
private static void SetState(TEntity entity, object state)
for (int i = 0; i < state.Length; i++)
thePropertyInfos[i].SetValue(entity, state[i]);
private bool HasChanges(TEntity entity)
var currentState = GetState(entity);
var previousState = myStates[entity];
for (int i = 0; i < currentState.Length; i++)
return false;
public IEnumerable<TEntity> GetEntities(EntityStates states)
if (states.HasFlag(EntityStates.New))
foreach (var entity in myNewEntities)
yield return entity;
if (states.HasFlag(EntityStates.Deleted))
foreach (var entity in myDeletedEntites)
yield return entity;
if (!states.HasFlag(EntityStates.Modified) &&
!states.HasFlag(EntityStates.Unmodified))
yield break;
foreach (var entity in myEntities.Where(e => !myNewEntities.Contains(e)))
var isModified = HasChanges(entity);
if (states.HasFlag(EntityStates.Modified) && isModified)
yield return entity;
else if (states.HasFlag(EntityStates.Unmodified) && !isModified)
yield return entity;
[Flags]
public enum EntityStates
New = 1,
Deleted = 2,
Modified = 4,
Unmodified = 8,
All = ~0,
Usage:
public class TestItem
public string Property get; set;
var changeTracker = new ChangeTracker<TestItem>();
var item1 = new TestItem();
var item2 = new TestItem();
var item3 = new TestItem();
var item4 = new TestItem();
var item5 = new TestItem();
changeTracker.Add(item1);
changeTracker.Add(item2);
changeTracker.Add(item3);
changeTracker.AcceptChanges();
changeTracker.Add(item4);
changeTracker.Add(item5);
item2.Property = "test";
changeTracker.Delete(item3);
changeTracker.Delete(item5);
Assert.IsTrue(changeTracker.HasChanges());
Assert.AreEqual(4, changeTracker.GetEntities(EntityStates.All).Count());
Assert.AreEqual(item1, changeTracker.GetEntities(EntityStates.Unmodified).Single());
Assert.AreEqual(item2, changeTracker.GetEntities(EntityStates.Modified).Single());
Assert.AreEqual(item3, changeTracker.GetEntities(EntityStates.Deleted).Single());
Assert.AreEqual(item4, changeTracker.GetEntities(EntityStates.New).Single());
Any feedback is welcome. However, I am specially intrested in alternative more performant solutions (without the overhead of boxing/unboxing and reflection).
[Edit]
I also tried binary serialization as "state storing strategy", but it is much slower (~100-200%) and it has the downside that the entity classes have to be marked as serializable.
c# .net
 |Â
show 5 more comments
up vote
5
down vote
favorite
Problem
I have a list of POCO model objects (approximatly 1.000 to 10.000) and I want to track changes on them:
- Check if at least one of the items has changed (to show the user that something changed)
- Get all new, deleted and modified objects to update the state in the database
For the first version, it is enough to support POCOs with primitive property types. However, probably I'll extend it in future to support complex property types.
Solution
public class ChangeTracker<TEntity> where TEntity : class
private static readonly PropertyInfo thePropertyInfos;
private readonly List<TEntity> myEntities = new List<TEntity>();
private readonly List<TEntity> myDeletedEntites = new List<TEntity>();
private readonly HashSet<TEntity> myNewEntities = new HashSet<TEntity>();
private readonly Dictionary<TEntity, object> myStates = new Dictionary<TEntity, object>();
static ChangeTracker()
thePropertyInfos = typeof(TEntity).GetProperties().ToArray();
public void Add(TEntity entity)
if (myStates.ContainsKey(entity))
throw new InvalidOperationException("It is not possible to add an entity twice.");
myStates.Add(entity, new object[thePropertyInfos.Length]);
myNewEntities.Add(entity);
myEntities.Add(entity);
public void Delete(TEntity entity)
myEntities.Remove(entity);
if (!myNewEntities.Remove(entity))
myDeletedEntites.Add(entity);
public void AcceptChanges()
myNewEntities.Clear();
myDeletedEntites.Clear();
foreach (var entity in myEntities)
myStates[entity] = GetState(entity);
public bool HasChanges()
EntityStates.Modified
private static object GetState(TEntity entity)
return thePropertyInfos
.Select(pi => pi.GetValue(entity))
.ToArray();
private static void SetState(TEntity entity, object state)
for (int i = 0; i < state.Length; i++)
thePropertyInfos[i].SetValue(entity, state[i]);
private bool HasChanges(TEntity entity)
var currentState = GetState(entity);
var previousState = myStates[entity];
for (int i = 0; i < currentState.Length; i++)
return false;
public IEnumerable<TEntity> GetEntities(EntityStates states)
if (states.HasFlag(EntityStates.New))
foreach (var entity in myNewEntities)
yield return entity;
if (states.HasFlag(EntityStates.Deleted))
foreach (var entity in myDeletedEntites)
yield return entity;
if (!states.HasFlag(EntityStates.Modified) &&
!states.HasFlag(EntityStates.Unmodified))
yield break;
foreach (var entity in myEntities.Where(e => !myNewEntities.Contains(e)))
var isModified = HasChanges(entity);
if (states.HasFlag(EntityStates.Modified) && isModified)
yield return entity;
else if (states.HasFlag(EntityStates.Unmodified) && !isModified)
yield return entity;
[Flags]
public enum EntityStates
New = 1,
Deleted = 2,
Modified = 4,
Unmodified = 8,
All = ~0,
Usage:
public class TestItem
public string Property get; set;
var changeTracker = new ChangeTracker<TestItem>();
var item1 = new TestItem();
var item2 = new TestItem();
var item3 = new TestItem();
var item4 = new TestItem();
var item5 = new TestItem();
changeTracker.Add(item1);
changeTracker.Add(item2);
changeTracker.Add(item3);
changeTracker.AcceptChanges();
changeTracker.Add(item4);
changeTracker.Add(item5);
item2.Property = "test";
changeTracker.Delete(item3);
changeTracker.Delete(item5);
Assert.IsTrue(changeTracker.HasChanges());
Assert.AreEqual(4, changeTracker.GetEntities(EntityStates.All).Count());
Assert.AreEqual(item1, changeTracker.GetEntities(EntityStates.Unmodified).Single());
Assert.AreEqual(item2, changeTracker.GetEntities(EntityStates.Modified).Single());
Assert.AreEqual(item3, changeTracker.GetEntities(EntityStates.Deleted).Single());
Assert.AreEqual(item4, changeTracker.GetEntities(EntityStates.New).Single());
Any feedback is welcome. However, I am specially intrested in alternative more performant solutions (without the overhead of boxing/unboxing and reflection).
[Edit]
I also tried binary serialization as "state storing strategy", but it is much slower (~100-200%) and it has the downside that the entity classes have to be marked as serializable.
c# .net
thePropertyInfos
- a new prefix? ;-D
â t3chb0t
Jan 26 at 20:50
no, it's the old one for static fields. The prefix for instance fields is still 'my' ;)
â JanDotNet
Jan 26 at 21:00
One more question, this time a technical one... why isEntityStates
aFlags
enum? It doesn't make much sense that an entity can have more then one state at the same time.
â t3chb0t
Jan 26 at 21:17
Becuase it allows to get e.g. all modified, new and deleted entites by combining the enum values:changeTracker.GetEntities(EntityStates.Modified | EntityStates.New | EntityStates.Deleted)
â JanDotNet
Jan 26 at 21:20
Further more, the enum is not used to associate the entity with a state - just to get entites for special states.
â JanDotNet
Jan 26 at 21:21
 |Â
show 5 more comments
up vote
5
down vote
favorite
up vote
5
down vote
favorite
Problem
I have a list of POCO model objects (approximatly 1.000 to 10.000) and I want to track changes on them:
- Check if at least one of the items has changed (to show the user that something changed)
- Get all new, deleted and modified objects to update the state in the database
For the first version, it is enough to support POCOs with primitive property types. However, probably I'll extend it in future to support complex property types.
Solution
public class ChangeTracker<TEntity> where TEntity : class
private static readonly PropertyInfo thePropertyInfos;
private readonly List<TEntity> myEntities = new List<TEntity>();
private readonly List<TEntity> myDeletedEntites = new List<TEntity>();
private readonly HashSet<TEntity> myNewEntities = new HashSet<TEntity>();
private readonly Dictionary<TEntity, object> myStates = new Dictionary<TEntity, object>();
static ChangeTracker()
thePropertyInfos = typeof(TEntity).GetProperties().ToArray();
public void Add(TEntity entity)
if (myStates.ContainsKey(entity))
throw new InvalidOperationException("It is not possible to add an entity twice.");
myStates.Add(entity, new object[thePropertyInfos.Length]);
myNewEntities.Add(entity);
myEntities.Add(entity);
public void Delete(TEntity entity)
myEntities.Remove(entity);
if (!myNewEntities.Remove(entity))
myDeletedEntites.Add(entity);
public void AcceptChanges()
myNewEntities.Clear();
myDeletedEntites.Clear();
foreach (var entity in myEntities)
myStates[entity] = GetState(entity);
public bool HasChanges()
EntityStates.Modified
private static object GetState(TEntity entity)
return thePropertyInfos
.Select(pi => pi.GetValue(entity))
.ToArray();
private static void SetState(TEntity entity, object state)
for (int i = 0; i < state.Length; i++)
thePropertyInfos[i].SetValue(entity, state[i]);
private bool HasChanges(TEntity entity)
var currentState = GetState(entity);
var previousState = myStates[entity];
for (int i = 0; i < currentState.Length; i++)
return false;
public IEnumerable<TEntity> GetEntities(EntityStates states)
if (states.HasFlag(EntityStates.New))
foreach (var entity in myNewEntities)
yield return entity;
if (states.HasFlag(EntityStates.Deleted))
foreach (var entity in myDeletedEntites)
yield return entity;
if (!states.HasFlag(EntityStates.Modified) &&
!states.HasFlag(EntityStates.Unmodified))
yield break;
foreach (var entity in myEntities.Where(e => !myNewEntities.Contains(e)))
var isModified = HasChanges(entity);
if (states.HasFlag(EntityStates.Modified) && isModified)
yield return entity;
else if (states.HasFlag(EntityStates.Unmodified) && !isModified)
yield return entity;
[Flags]
public enum EntityStates
New = 1,
Deleted = 2,
Modified = 4,
Unmodified = 8,
All = ~0,
Usage:
public class TestItem
public string Property get; set;
var changeTracker = new ChangeTracker<TestItem>();
var item1 = new TestItem();
var item2 = new TestItem();
var item3 = new TestItem();
var item4 = new TestItem();
var item5 = new TestItem();
changeTracker.Add(item1);
changeTracker.Add(item2);
changeTracker.Add(item3);
changeTracker.AcceptChanges();
changeTracker.Add(item4);
changeTracker.Add(item5);
item2.Property = "test";
changeTracker.Delete(item3);
changeTracker.Delete(item5);
Assert.IsTrue(changeTracker.HasChanges());
Assert.AreEqual(4, changeTracker.GetEntities(EntityStates.All).Count());
Assert.AreEqual(item1, changeTracker.GetEntities(EntityStates.Unmodified).Single());
Assert.AreEqual(item2, changeTracker.GetEntities(EntityStates.Modified).Single());
Assert.AreEqual(item3, changeTracker.GetEntities(EntityStates.Deleted).Single());
Assert.AreEqual(item4, changeTracker.GetEntities(EntityStates.New).Single());
Any feedback is welcome. However, I am specially intrested in alternative more performant solutions (without the overhead of boxing/unboxing and reflection).
[Edit]
I also tried binary serialization as "state storing strategy", but it is much slower (~100-200%) and it has the downside that the entity classes have to be marked as serializable.
c# .net
Problem
I have a list of POCO model objects (approximatly 1.000 to 10.000) and I want to track changes on them:
- Check if at least one of the items has changed (to show the user that something changed)
- Get all new, deleted and modified objects to update the state in the database
For the first version, it is enough to support POCOs with primitive property types. However, probably I'll extend it in future to support complex property types.
Solution
public class ChangeTracker<TEntity> where TEntity : class
private static readonly PropertyInfo thePropertyInfos;
private readonly List<TEntity> myEntities = new List<TEntity>();
private readonly List<TEntity> myDeletedEntites = new List<TEntity>();
private readonly HashSet<TEntity> myNewEntities = new HashSet<TEntity>();
private readonly Dictionary<TEntity, object> myStates = new Dictionary<TEntity, object>();
static ChangeTracker()
thePropertyInfos = typeof(TEntity).GetProperties().ToArray();
public void Add(TEntity entity)
if (myStates.ContainsKey(entity))
throw new InvalidOperationException("It is not possible to add an entity twice.");
myStates.Add(entity, new object[thePropertyInfos.Length]);
myNewEntities.Add(entity);
myEntities.Add(entity);
public void Delete(TEntity entity)
myEntities.Remove(entity);
if (!myNewEntities.Remove(entity))
myDeletedEntites.Add(entity);
public void AcceptChanges()
myNewEntities.Clear();
myDeletedEntites.Clear();
foreach (var entity in myEntities)
myStates[entity] = GetState(entity);
public bool HasChanges()
EntityStates.Modified
private static object GetState(TEntity entity)
return thePropertyInfos
.Select(pi => pi.GetValue(entity))
.ToArray();
private static void SetState(TEntity entity, object state)
for (int i = 0; i < state.Length; i++)
thePropertyInfos[i].SetValue(entity, state[i]);
private bool HasChanges(TEntity entity)
var currentState = GetState(entity);
var previousState = myStates[entity];
for (int i = 0; i < currentState.Length; i++)
return false;
public IEnumerable<TEntity> GetEntities(EntityStates states)
if (states.HasFlag(EntityStates.New))
foreach (var entity in myNewEntities)
yield return entity;
if (states.HasFlag(EntityStates.Deleted))
foreach (var entity in myDeletedEntites)
yield return entity;
if (!states.HasFlag(EntityStates.Modified) &&
!states.HasFlag(EntityStates.Unmodified))
yield break;
foreach (var entity in myEntities.Where(e => !myNewEntities.Contains(e)))
var isModified = HasChanges(entity);
if (states.HasFlag(EntityStates.Modified) && isModified)
yield return entity;
else if (states.HasFlag(EntityStates.Unmodified) && !isModified)
yield return entity;
[Flags]
public enum EntityStates
New = 1,
Deleted = 2,
Modified = 4,
Unmodified = 8,
All = ~0,
Usage:
public class TestItem
public string Property get; set;
var changeTracker = new ChangeTracker<TestItem>();
var item1 = new TestItem();
var item2 = new TestItem();
var item3 = new TestItem();
var item4 = new TestItem();
var item5 = new TestItem();
changeTracker.Add(item1);
changeTracker.Add(item2);
changeTracker.Add(item3);
changeTracker.AcceptChanges();
changeTracker.Add(item4);
changeTracker.Add(item5);
item2.Property = "test";
changeTracker.Delete(item3);
changeTracker.Delete(item5);
Assert.IsTrue(changeTracker.HasChanges());
Assert.AreEqual(4, changeTracker.GetEntities(EntityStates.All).Count());
Assert.AreEqual(item1, changeTracker.GetEntities(EntityStates.Unmodified).Single());
Assert.AreEqual(item2, changeTracker.GetEntities(EntityStates.Modified).Single());
Assert.AreEqual(item3, changeTracker.GetEntities(EntityStates.Deleted).Single());
Assert.AreEqual(item4, changeTracker.GetEntities(EntityStates.New).Single());
Any feedback is welcome. However, I am specially intrested in alternative more performant solutions (without the overhead of boxing/unboxing and reflection).
[Edit]
I also tried binary serialization as "state storing strategy", but it is much slower (~100-200%) and it has the downside that the entity classes have to be marked as serializable.
c# .net
edited Jan 27 at 20:11
asked Jan 26 at 20:48
JanDotNet
6,4011238
6,4011238
thePropertyInfos
- a new prefix? ;-D
â t3chb0t
Jan 26 at 20:50
no, it's the old one for static fields. The prefix for instance fields is still 'my' ;)
â JanDotNet
Jan 26 at 21:00
One more question, this time a technical one... why isEntityStates
aFlags
enum? It doesn't make much sense that an entity can have more then one state at the same time.
â t3chb0t
Jan 26 at 21:17
Becuase it allows to get e.g. all modified, new and deleted entites by combining the enum values:changeTracker.GetEntities(EntityStates.Modified | EntityStates.New | EntityStates.Deleted)
â JanDotNet
Jan 26 at 21:20
Further more, the enum is not used to associate the entity with a state - just to get entites for special states.
â JanDotNet
Jan 26 at 21:21
 |Â
show 5 more comments
thePropertyInfos
- a new prefix? ;-D
â t3chb0t
Jan 26 at 20:50
no, it's the old one for static fields. The prefix for instance fields is still 'my' ;)
â JanDotNet
Jan 26 at 21:00
One more question, this time a technical one... why isEntityStates
aFlags
enum? It doesn't make much sense that an entity can have more then one state at the same time.
â t3chb0t
Jan 26 at 21:17
Becuase it allows to get e.g. all modified, new and deleted entites by combining the enum values:changeTracker.GetEntities(EntityStates.Modified | EntityStates.New | EntityStates.Deleted)
â JanDotNet
Jan 26 at 21:20
Further more, the enum is not used to associate the entity with a state - just to get entites for special states.
â JanDotNet
Jan 26 at 21:21
thePropertyInfos
- a new prefix? ;-Dâ t3chb0t
Jan 26 at 20:50
thePropertyInfos
- a new prefix? ;-Dâ t3chb0t
Jan 26 at 20:50
no, it's the old one for static fields. The prefix for instance fields is still 'my' ;)
â JanDotNet
Jan 26 at 21:00
no, it's the old one for static fields. The prefix for instance fields is still 'my' ;)
â JanDotNet
Jan 26 at 21:00
One more question, this time a technical one... why is
EntityStates
a Flags
enum? It doesn't make much sense that an entity can have more then one state at the same time.â t3chb0t
Jan 26 at 21:17
One more question, this time a technical one... why is
EntityStates
a Flags
enum? It doesn't make much sense that an entity can have more then one state at the same time.â t3chb0t
Jan 26 at 21:17
Becuase it allows to get e.g. all modified, new and deleted entites by combining the enum values:
changeTracker.GetEntities(EntityStates.Modified | EntityStates.New | EntityStates.Deleted)
â JanDotNet
Jan 26 at 21:20
Becuase it allows to get e.g. all modified, new and deleted entites by combining the enum values:
changeTracker.GetEntities(EntityStates.Modified | EntityStates.New | EntityStates.Deleted)
â JanDotNet
Jan 26 at 21:20
Further more, the enum is not used to associate the entity with a state - just to get entites for special states.
â JanDotNet
Jan 26 at 21:21
Further more, the enum is not used to associate the entity with a state - just to get entites for special states.
â JanDotNet
Jan 26 at 21:21
 |Â
show 5 more comments
1 Answer
1
active
oldest
votes
up vote
2
down vote
accepted
State arrays
These confuse me rather. I'm going to suggest you don't add an 'empty' array in Add
, and instead add null
.
myStates.Add(entity, null);
The empty array is a valid state, and as far as I can tell you want this to be treated as a non-set value; null
is not, and will cause a violent crash if you try to treat it as such (which is good).
This sort of requires a change to HasChanges
(though I don't think it can be called before on a new entity prior to AcceptChanges
), to explicitly check if the previousState
is null
(which could have some performance benefits, but who cares). Of course, you could use some other explicit sentinel type (i.e. do it properly), or just not add anything to the myStates
dictionary until such a state exists (has all the benefit of null
without any of the fear).
GetEntities
GetEntities
is a bit painful to read. I would be inclined to keep new entities in myNewEntities
until they are accepted (much cheaper in the case where the changes are rejected), just as deleted entities to not appear in MyEntities
. This will simplify the implementation of GetEntities
, removing the check for new entities.
You also aren't clearing states stored for deleted entities (should be done in AcceptChanges
).
Misc
Not a big issue, but I would consider making
thePropertyInfos
anIReadOnlyList<TEntity>
(though I probably wouldn't effect this, since hopefully no-one will ever look at this code).This could do with some inline (
\
) documentation for the external API, and some indication that it is not thread safe.Is there any particular reason why
myEntities
is aList
and not aHashSet
? I'd be somewhat inclined to ditch the list completely and just keep the State dictionary (no redundancy -> less effort to maintain)No
RejectChanges
? Without this, certain bits can be simplified considerably (e.g. ditch state as soon as you delete something).
Thanks for the great feedback, your points are valid and I have changed the implementation as suggested in 'state arrays' and 'GetEntities'. The problem with RejectChange is, that the application holds it's own list of entities. If entites were deleted, reject changes would not restore the entites in the application's list but revert changes of the non-deleted entites. Therefore I decide to not support RejectChanges .
â JanDotNet
Jan 27 at 19:40
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
2
down vote
accepted
State arrays
These confuse me rather. I'm going to suggest you don't add an 'empty' array in Add
, and instead add null
.
myStates.Add(entity, null);
The empty array is a valid state, and as far as I can tell you want this to be treated as a non-set value; null
is not, and will cause a violent crash if you try to treat it as such (which is good).
This sort of requires a change to HasChanges
(though I don't think it can be called before on a new entity prior to AcceptChanges
), to explicitly check if the previousState
is null
(which could have some performance benefits, but who cares). Of course, you could use some other explicit sentinel type (i.e. do it properly), or just not add anything to the myStates
dictionary until such a state exists (has all the benefit of null
without any of the fear).
GetEntities
GetEntities
is a bit painful to read. I would be inclined to keep new entities in myNewEntities
until they are accepted (much cheaper in the case where the changes are rejected), just as deleted entities to not appear in MyEntities
. This will simplify the implementation of GetEntities
, removing the check for new entities.
You also aren't clearing states stored for deleted entities (should be done in AcceptChanges
).
Misc
Not a big issue, but I would consider making
thePropertyInfos
anIReadOnlyList<TEntity>
(though I probably wouldn't effect this, since hopefully no-one will ever look at this code).This could do with some inline (
\
) documentation for the external API, and some indication that it is not thread safe.Is there any particular reason why
myEntities
is aList
and not aHashSet
? I'd be somewhat inclined to ditch the list completely and just keep the State dictionary (no redundancy -> less effort to maintain)No
RejectChanges
? Without this, certain bits can be simplified considerably (e.g. ditch state as soon as you delete something).
Thanks for the great feedback, your points are valid and I have changed the implementation as suggested in 'state arrays' and 'GetEntities'. The problem with RejectChange is, that the application holds it's own list of entities. If entites were deleted, reject changes would not restore the entites in the application's list but revert changes of the non-deleted entites. Therefore I decide to not support RejectChanges .
â JanDotNet
Jan 27 at 19:40
add a comment |Â
up vote
2
down vote
accepted
State arrays
These confuse me rather. I'm going to suggest you don't add an 'empty' array in Add
, and instead add null
.
myStates.Add(entity, null);
The empty array is a valid state, and as far as I can tell you want this to be treated as a non-set value; null
is not, and will cause a violent crash if you try to treat it as such (which is good).
This sort of requires a change to HasChanges
(though I don't think it can be called before on a new entity prior to AcceptChanges
), to explicitly check if the previousState
is null
(which could have some performance benefits, but who cares). Of course, you could use some other explicit sentinel type (i.e. do it properly), or just not add anything to the myStates
dictionary until such a state exists (has all the benefit of null
without any of the fear).
GetEntities
GetEntities
is a bit painful to read. I would be inclined to keep new entities in myNewEntities
until they are accepted (much cheaper in the case where the changes are rejected), just as deleted entities to not appear in MyEntities
. This will simplify the implementation of GetEntities
, removing the check for new entities.
You also aren't clearing states stored for deleted entities (should be done in AcceptChanges
).
Misc
Not a big issue, but I would consider making
thePropertyInfos
anIReadOnlyList<TEntity>
(though I probably wouldn't effect this, since hopefully no-one will ever look at this code).This could do with some inline (
\
) documentation for the external API, and some indication that it is not thread safe.Is there any particular reason why
myEntities
is aList
and not aHashSet
? I'd be somewhat inclined to ditch the list completely and just keep the State dictionary (no redundancy -> less effort to maintain)No
RejectChanges
? Without this, certain bits can be simplified considerably (e.g. ditch state as soon as you delete something).
Thanks for the great feedback, your points are valid and I have changed the implementation as suggested in 'state arrays' and 'GetEntities'. The problem with RejectChange is, that the application holds it's own list of entities. If entites were deleted, reject changes would not restore the entites in the application's list but revert changes of the non-deleted entites. Therefore I decide to not support RejectChanges .
â JanDotNet
Jan 27 at 19:40
add a comment |Â
up vote
2
down vote
accepted
up vote
2
down vote
accepted
State arrays
These confuse me rather. I'm going to suggest you don't add an 'empty' array in Add
, and instead add null
.
myStates.Add(entity, null);
The empty array is a valid state, and as far as I can tell you want this to be treated as a non-set value; null
is not, and will cause a violent crash if you try to treat it as such (which is good).
This sort of requires a change to HasChanges
(though I don't think it can be called before on a new entity prior to AcceptChanges
), to explicitly check if the previousState
is null
(which could have some performance benefits, but who cares). Of course, you could use some other explicit sentinel type (i.e. do it properly), or just not add anything to the myStates
dictionary until such a state exists (has all the benefit of null
without any of the fear).
GetEntities
GetEntities
is a bit painful to read. I would be inclined to keep new entities in myNewEntities
until they are accepted (much cheaper in the case where the changes are rejected), just as deleted entities to not appear in MyEntities
. This will simplify the implementation of GetEntities
, removing the check for new entities.
You also aren't clearing states stored for deleted entities (should be done in AcceptChanges
).
Misc
Not a big issue, but I would consider making
thePropertyInfos
anIReadOnlyList<TEntity>
(though I probably wouldn't effect this, since hopefully no-one will ever look at this code).This could do with some inline (
\
) documentation for the external API, and some indication that it is not thread safe.Is there any particular reason why
myEntities
is aList
and not aHashSet
? I'd be somewhat inclined to ditch the list completely and just keep the State dictionary (no redundancy -> less effort to maintain)No
RejectChanges
? Without this, certain bits can be simplified considerably (e.g. ditch state as soon as you delete something).
State arrays
These confuse me rather. I'm going to suggest you don't add an 'empty' array in Add
, and instead add null
.
myStates.Add(entity, null);
The empty array is a valid state, and as far as I can tell you want this to be treated as a non-set value; null
is not, and will cause a violent crash if you try to treat it as such (which is good).
This sort of requires a change to HasChanges
(though I don't think it can be called before on a new entity prior to AcceptChanges
), to explicitly check if the previousState
is null
(which could have some performance benefits, but who cares). Of course, you could use some other explicit sentinel type (i.e. do it properly), or just not add anything to the myStates
dictionary until such a state exists (has all the benefit of null
without any of the fear).
GetEntities
GetEntities
is a bit painful to read. I would be inclined to keep new entities in myNewEntities
until they are accepted (much cheaper in the case where the changes are rejected), just as deleted entities to not appear in MyEntities
. This will simplify the implementation of GetEntities
, removing the check for new entities.
You also aren't clearing states stored for deleted entities (should be done in AcceptChanges
).
Misc
Not a big issue, but I would consider making
thePropertyInfos
anIReadOnlyList<TEntity>
(though I probably wouldn't effect this, since hopefully no-one will ever look at this code).This could do with some inline (
\
) documentation for the external API, and some indication that it is not thread safe.Is there any particular reason why
myEntities
is aList
and not aHashSet
? I'd be somewhat inclined to ditch the list completely and just keep the State dictionary (no redundancy -> less effort to maintain)No
RejectChanges
? Without this, certain bits can be simplified considerably (e.g. ditch state as soon as you delete something).
answered Jan 27 at 11:11
VisualMelon
2,280716
2,280716
Thanks for the great feedback, your points are valid and I have changed the implementation as suggested in 'state arrays' and 'GetEntities'. The problem with RejectChange is, that the application holds it's own list of entities. If entites were deleted, reject changes would not restore the entites in the application's list but revert changes of the non-deleted entites. Therefore I decide to not support RejectChanges .
â JanDotNet
Jan 27 at 19:40
add a comment |Â
Thanks for the great feedback, your points are valid and I have changed the implementation as suggested in 'state arrays' and 'GetEntities'. The problem with RejectChange is, that the application holds it's own list of entities. If entites were deleted, reject changes would not restore the entites in the application's list but revert changes of the non-deleted entites. Therefore I decide to not support RejectChanges .
â JanDotNet
Jan 27 at 19:40
Thanks for the great feedback, your points are valid and I have changed the implementation as suggested in 'state arrays' and 'GetEntities'. The problem with RejectChange is, that the application holds it's own list of entities. If entites were deleted, reject changes would not restore the entites in the application's list but revert changes of the non-deleted entites. Therefore I decide to not support RejectChanges .
â JanDotNet
Jan 27 at 19:40
Thanks for the great feedback, your points are valid and I have changed the implementation as suggested in 'state arrays' and 'GetEntities'. The problem with RejectChange is, that the application holds it's own list of entities. If entites were deleted, reject changes would not restore the entites in the application's list but revert changes of the non-deleted entites. Therefore I decide to not support RejectChanges .
â JanDotNet
Jan 27 at 19:40
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186086%2fsimple-change-tracker-for-pocos%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
thePropertyInfos
- a new prefix? ;-Dâ t3chb0t
Jan 26 at 20:50
no, it's the old one for static fields. The prefix for instance fields is still 'my' ;)
â JanDotNet
Jan 26 at 21:00
One more question, this time a technical one... why is
EntityStates
aFlags
enum? It doesn't make much sense that an entity can have more then one state at the same time.â t3chb0t
Jan 26 at 21:17
Becuase it allows to get e.g. all modified, new and deleted entites by combining the enum values:
changeTracker.GetEntities(EntityStates.Modified | EntityStates.New | EntityStates.Deleted)
â JanDotNet
Jan 26 at 21:20
Further more, the enum is not used to associate the entity with a state - just to get entites for special states.
â JanDotNet
Jan 26 at 21:21