views:

1376

answers:

9

The following is a simple example of an enum which defines the state of an object and a class which shows the implementation of this enum.

public enum StatusEnum
{
    Clean = 0,
    Dirty = 1,
    New = 2,
    Deleted = 3,
    Purged = 4
}


public class Example_Class
{
    private StatusEnum _Status = StatusEnum.New;

    private long _ID;
    private string _Name;

    public StatusEnum Status
    {
        get { return _Status; }
        set { _Status = value; }
    }

    public long ID
    {
        get { return _ID; }
        set { _ID = value; }
    }

    public string Name
    {
        get { return _Name; }
        set { _Name = value; }
    }
}

when populating the class object with data from the database, we set the enum value to "clean". with the goal of keeping most of the logic out of the presentation layer, how can we set the enum value to "dirty" when a property is changed.

i was thinking something along the lines of;

public string Name
{
    get { return _Name; }
    set 
    {
     if (value != _Name)
     {
               _Name = value; 
        _Status = StatusEnum.Dirty;
     }
    } 
}

in the setter of each property of the class.

does this sound like a good idea, does anyone have any better ideas on how the dirty flag can be assigned without doing so in the presentation layer.

+12  A: 

One option is to change it on write; another is to keep a copy of all the original values and compute the dirtiness when anyone asks for it. That has the added benefit that you can tell exactly which fields have changed (and in what way) which means you can issue minimal update statements and make merge conflict resolution slightly easier.

You also get to put all the dirtiness-checking in one place, so it doesn't pollute the rest of your code.

I'm not saying it's perfect, but it's an option worth considering.

Jon Skeet
+1 I've implemented this in the past and I ended up hating it. It's a maintenance nightmare. DirtyFlag should become an anti-pattern IMHO. If you implement this, you lose Auto Properties and your code bloats immensely
Chad Grant
+1  A: 

Your approach is basically how I would do it. I would just remove the setter for the Status property:

public StatusEnum Status
{
    get { return _Status; }
    // set { _Status = value; }
}

and instead add a function

public SetStatusClean()
{
    _Status = StatusEnum.Clean;
}

As well as SetStatusDeleted() and SetStatusPurged(), because I find it better indicates the intention.

Edit

Having read the answer by Jon Skeet, I need to reconsider my approach ;-) For simple objects I would stick with my way, but if it gets more complex, his proposal would lead to much better organised code.

Treb
Your edit almost made me vote for your answer :)
Torbjørn
A: 

If your Example_Class is lightweight, consider storing the original state and then comparing the current state to the original in order to determine the changes. If not your approach is the best because stroing the original state consumes a lot of system resources in this case.

M. Jahedbozorgan
Depending of the case, this could be a very good solution. It allows a general implementation that does not "pollute" the domain classes with such things. It is also how NHibernate does dirty checks. I wouldn't care *too* much about system resources in this case.
Stefan Steinegger
+9  A: 

When you really do want a dirty flag at the class level (or, for that matter, notifications) - you can use tricks like below to minimise the clutter in your properties (here showing both IsDirty and PropertyChanged, just for fun).

Obviously it is a trivial matter to use the enum approach (the only reason I didn't was to keep the example simple):

class SomeType : INotifyPropertyChanged {
    private int foo;
    public int Foo {
        get { return foo; }
        set { SetField(ref foo, value, "Foo"); }
    }

    private string bar;
    public string Bar {
        get { return bar; }
        set { SetField(ref bar, value, "Bar"); }
    }

    public bool IsDirty { get; private set; }
    public event PropertyChangedEventHandler PropertyChanged;
    protected void SetField<T>(ref T field, T value, string propertyName) {
        if (!EqualityComparer<T>.Default.Equals(field, value)) {
            field = value;
            IsDirty = true;
            OnPropertyChanged(propertyName);
        }
    }
    protected virtual void OnPropertyChanged(string propertyName) {
        var handler = PropertyChanged;
        if (handler != null) {
            handler(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}

You might also choose to push some of that into an abstract base class, but that is a separate discussion

Marc Gravell
+1: I used exactly the same in some projects (without the OnPropertyChanged thing, as I didn't care about it) as an improvement on doing the equality check in every property. Definitely worth trying.
Dan C.
A: 

Another method is to override the GetHashCode() method to somthing like this:

public override int GetHashCode() // or call it GetChangeHash or somthing if you dont want to override the GetHashCode function...
{
    var sb = new System.Text.StringBuilder();

    sb.Append(_dateOfBirth);
    sb.Append(_marital);
    sb.Append(_gender);
    sb.Append(_notes);
    sb.Append(_firstName);
    sb.Append(_lastName);  

    return sb.ToString.GetHashCode();
}

Once loaded from the database, get the hash code of the object. Then just before you save check if the current hash code is equal to the previous hash code. if they are the same, don't save.

Edit:

As people have pointed out this causes the hash code to change - as i use Guids to identify my objects, i don't mind if the hashcode changes.

Edit2:

Since people are adverse to changing the hash code, instead of overriding the GetHashCode method, just call the method something else. The point is detecting a change not whether i use guids or hashcodes for object identification.

Pondidum
I don't think that this is a good idea, since the Hashcode of an object should not change during the lifetime of the object.Suppose you store your objects for some reason in a Hashtable or Dictionary (and the object is the key), you will not be able to find the object back when the hashcode has changed.
Frederik Gheysels
We use guids for each object so this is no problem for us.
Pondidum
Why should this avoid the problem ? When you create your hashcode based on changeable properties, then your hashcode will always change when you change a property.
Frederik Gheysels
We only use Guids as our key when the objects are in hashtables or in dictionaries, so like i said its no problem if the hash code changes.
Pondidum
+5  A: 

If you want to implement it in this way, and you want to reduce the amount of code, you might consider applying Aspect Oriented Programming.

You can for instance use a compile-time weaver like PostSharp , and create an 'aspect' that can be applied to properties. This aspect then makes sure that your dirty flag is set when appropriate.

The aspect can look like this:

[Serializable]
[AttributeUsage(AttributeTargets.Property)]
public class ChangeTrackingAttribute : OnMethodInvocationAspect
{
    public override void OnInvocation( MethodInvocationEventArgs e )
    {
        if( e.Delegate.Method.ReturnParameter.ParameterType == typeof(void) )
        {
              // we're in the setter
              IChangeTrackable target = e.Delegate.Target as IChangeTrackable;

              // Implement some logic to retrieve the current value of 
              // the property
              if( currentValue != e.GetArgumentArray()[0] )
              {
                  target.Status = Status.Dirty;
              }
              base.OnInvocation (e);
        } 
    }  
}

Offcourse, this means that the classes for which you want to implement ChangeTracking, should implement the IChangeTrackable interface (custom interface), which has at least the 'Status' property.

You can also create a custom attribute ChangeTrackingProperty, and make sure that the aspect that has been created above, is only applied to properties that are decorated with this ChangeTrackingProperty attribute.

For instance:

public class Customer : IChangeTrackable
{
    public DirtyState Status
    {
        get; set;
    }

    [ChangeTrackingProperty]
    public string Name
    { get; set; }
}

This is a little bit how I see it. You can even make sure that PostSharp checks at compile-time whether classes that have properties that are decorated with the ChangeTrackingProperty attribute, implement the IChangeTrackable interface.

Frederik Gheysels
+3  A: 

Take a look at PostSharp (http://www.postsharp.org/). You can easily create a Attribute which marks it as dirty you can add the attrubute to each property that needs it and it keeps all your code in one place.

Roughly speaking Create an interface which has your status in make the class implement it. Create an attribute which can be applied on properties and cast to your interface in order to set the value when something changes one of the marked properties.

Saint Gerbil
A: 

Apart from the advice of 'consider making your type immutable', here's something I wrote up (and got Jon and Marc to teach me something along the way)

public class Example_Class
{    // snip
     // all properties are public get and private set

     private Dictionary<string, Delegate> m_PropertySetterMap;

     public Example_Class()
     {
        m_PropertySetterMap = new Dictionary<string, Delegate>();
        InitializeSettableProperties();
     }
     public Example_Class(long id, string name):this()
     {   this.ID = id;    this.Name = name;   }

     private void InitializeSettableProperties()
     {
        AddToPropertyMap<long>("ID",  value => { this.ID = value; });
        AddToPropertyMap<string>("Name", value => { this.Name = value; }); 
     }
     // jump thru a hoop because it won't let me cast an anonymous method to an Action<T>/Delegate
     private void AddToPropertyMap<T>(string sPropertyName, Action<T> setterAction)
     {   m_PropertySetterMap.Add(sPropertyName, setterAction);            }

     public void SetProperty<T>(string propertyName, T value)
     {
        (m_PropertySetterMap[propertyName] as Action<T>).Invoke(value);
        this.Status = StatusEnum.Dirty;
     }
  }

You get the idea.. possible improvements: Use constants for PropertyNames & check if property has really changed. One drawback here is that

obj.SetProperty("ID", 700);         // will blow up int instead of long
obj.SetProperty<long>("ID", 700);   // be explicit or use 700L
Gishu