Properties with events: another attempt
See also: Properties as objects
The goal here is to have a class with a property that implements the Observer pattern, based on a statement in Object Thinking that all classes should support this. (I’m not sure I agree, but I’m exploring it anyway.)
So far I’ve proposed two options:
Let the class support its own notifications of events. Downsides are:
- Duplicated code as soon as another property wants the same feature.
- 11 lines of text in my class to support the basic notion of an object with an attribute
- Possibility of a bug where the class directly modifies the backing store, without firing the OnSet event.
Create a class to do it all for you. Downsides are:
- Can’t limit access to the setter to just the containing class.
- Violates encapsulation rule of Law of Demeter by exposing an implementation detail (that I’m using Property<>).
Upon further discussion with members of the team, we’ve come up with something I like quite a bit. It balances responsibilities between the class you’re writing and the Property<> class. It seems to solve all the problems listed above.
Here’s the code:
interface IReadonlyProperty<T>
{
T Value { get; }
event Property<T>.SetEventHandler OnSet;
}
class Property<T> : IReadonlyProperty<T>
{
T _value;
public Property()
{
// put something into the event so that I don't have to check
// for null later.
this.OnSet += delegate { };
}
public delegate void SetEventHandler(object sender, T newValue);
public event SetEventHandler OnSet;
public T Value
{
get
{
return this._value;
}
set
{
this._value = value;
this.OnSet(this, this._value);
}
}
public void SetValueWithoutFiringEvent(T t) { this._value = t; }
T IReadonlyProperty<T>.Value { get { return this._value; } }
}
To demonstrate its worth, here’s the usage:
class Customer
{
Property<int> _age = new Property<int>();
public IReadonlyProperty<int> Age
{
get { return this._age; }
}
public void Test()
{
int eventFiredCount = 0;
this._age.OnSet += delegate { eventFiredCount++; };
Debug.Assert(eventFiredCount == 0);
this._age.Value = 9;
Debug.Assert(eventFiredCount == 1);
this._age.SetValueWithoutFiringEvent(7);
Debug.Assert(eventFiredCount == 1);
}
}
Note that I can still set the value internally to Customer without firing the event, but only by explicitly saying that’s what I’m doing. I could decide never to allow that by just removing the SetValueWithoutFiringEvent method.
Comments
- Anonymous
June 16, 2004
This looks like a clean solution. What concerns me in your code is that we still have to check for null when firing events. Why is that? - Anonymous
June 16, 2004
RJ: Because that's how C# works. If the event is empty, it will throw a NullReferenceException.
The guidance is to use:
EventHandler temp = this.MyEvent;
if (temp)
{
temp(this, ...);
}
But that's a whole lot of code for a very small problem. So instead I just jam in an empty delegate at the beginning. - Anonymous
June 16, 2004
I'm not done with the Object Thinking book yet, but I can now say that there are several of things that I don't like. In the book we can read that all objects should notify if its sate has been changed, the object should not know who the client are. I think this is against the whole Object thinking idea. For example, if I'm a person (object) and change my name, no one except the object that asks me for my name knows that my name has been changed. No one can get my name without asking me for it first. If every one can be notified when I change my name without my knowledge, is like someone is going right into my mind to get the information without going through me (asking me). That is why I do not agree with the author of the book that EVERY object should make it possible for other object to know when the state of the object is changed. - Anonymous
June 16, 2004
I agree with RJ, I never liked this event behavior. Event should have been smart enough to take care of it.
That said, having anonymous delegates makes life easier. - Anonymous
June 17, 2004
Jay,
Because the Customer class is returning the Property instance in the implementation of Age, I can cast to the base Property class and call those methods directly. Thus, calling the SetValueWithoutFiringEvent method is allowed in this scenario. - Anonymous
June 18, 2004
Phew, took me a while to get this. I'm probably not used to generics enough. :)
Nice, elegant solution. I would, however, get rid of the neccessity to explicitly use Property<T>.Value in the class itself by implementing an implicit cast for T/Property<T> and Property<T>/T.
Omer - Anonymous
June 18, 2004
Ron: I see the concern. One might argue that the situation is analagous to Reflection's violation of encapsulation.
I might agree with that argument, except that it's easy to know if your code does or doesn't have reflection. It's harder to know if anyone is doing a cast.
I could return a copy, I suppose.
Any other ideas? - Anonymous
June 18, 2004
Omer: The cast may be useful, but it never makes a Property<T> completely act the same as a T. It's a toss-up. - Anonymous
June 18, 2004
Jay,
I would rather have a protected virtual OnSet method and AfterSet event than just an OnSet event (also consistant with the naming convention ;).
This way, if I decided to derive from your Property<T> class, I would be able to extend the event invocation, in the same way Windows Forms controls expose their event invocations.
Another thing, which I posted back on the original post ("Properties as objects"), is that I would appreciate if you would send the old value as well as the new one. This way I would have the full picture as to the transformation that has happened to the property's value.
And maybe it's just me, but I don't like the event initialization with the anonymous delegate. :/
Omer - Anonymous
June 21, 2004
Omer, stuff in this code at the right locations:
public delegate void SetEventHandler(object sender, T oldValue, T newValue);
this.OnSet(this, this._value, value);
this._value = value;