current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite
1

I have the following code to log changes using Entity Framework 6.

At the moment it only logs changes to a particular table - but I intend to expand it to cope with any table. I'm not sure of the best way to extract the table name.

I call this code before Db.SaveChanges. Db is the DBContext.

private void LogHistory()
    {
        var entries = this.Db.ChangeTracker.Entries()
            .Where(e => e.State == EntityState.Added || e.State == EntityState.Modified || e.State == EntityState.Deleted)
            .ToList();
        foreach (var entry in entries)
        {
            foreach (string o in entry.CurrentValues.PropertyNames)
            {
                var property = entry.Property(o);
                var currentVal = property.CurrentValue == null ? "" : property.CurrentValue.ToString();
                var originalVal = property.OriginalValue == null ? "" : property.OriginalValue.ToString();

                if (currentVal != originalVal)
                {
                    if (entry.Entity.GetType().Name.Contains("PropetyPair"))
                    {
                        //  make and add log record
                    }
                }
            }
        }
    }
share|improve this question
    
I'm not an entity framework guru but I would have thought you might inherit the context and override SaveChanges() ? –  James Khoury Jan 16 at 23:56
    
Yes I actually do that, but i left it out for simplicity, since I don't think it is relevant to my question. –  kirsten g Jan 17 at 0:53
    
You may find FrameLog useful. It is a general purpose EF logging library. Either as an alternative to writing this yourself or as a source of code to compare with (it is open source). –  Martin Eden 2 days ago
add comment

1 Answer

up vote 5 down vote accepted

Not a big deal, but this:

var currentVal = property.CurrentValue == null ? "" : property.CurrentValue.ToString();
var originalVal = property.OriginalValue == null ? "" : property.OriginalValue.ToString();

Can be shortened to this:

var currentVal = (property.CurrentValue ?? string.Empty).ToString();
var originalVal = (property.OriginalValue ?? string.Empty).ToString();

I'm not sure of the best way to extract the table name.

You're not doing that. You're looking at the name of the type of the entity - being an object/relational mapper, EF maps that entity to a table; if you need the table name then you need to look at the entity's table mappings.

I intend to expand it to cope with any table

How about having an IEnumerable<Type> where you have all the entity types you want to log changes for, and then instead of getting the type's name and comparing with some magic string, you can just do _monitoredTypes.Contains(typeof(entry.Entity)) (not tested, just a thought).

In terms of , you're looping too much:

if (entry.Entity.GetType().Name.Contains("PropetyPair"))

This condition doesn't need string o, as it's working off entry - thus, you can move that condition up two levels and only enter the 2nd loop when the entity type is interesting.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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