views:

38

answers:

1

Considering the code below:

public class TableMain {
    public virtual event Action UpdateFilter;
....
}

public class TableSub : TableMain {
   public override event Action UpdateFilter;

   public void UpdateQuery(){ 
    .....
    if(UpdateFilter!=null){
          UpdateFilter(); // Invocation of polymorphic field-like event???
}
}
}

In this code resharper shows the alert "invocation of polymorphic like event".

My question is: What does it actually mean? and is it an alert for a bad programming practice? Also, is it a bad practice to call an event polymorphically? ( Knowing that an event can only be raised from the class which has declared it)

Thanks for your interest.

+2  A: 

Well, you've actually got two field-like events here. Your override will be overriding the add/remove part, but you'll have two fields - one in TableMain and one in TableSub. Only the one in TableSub will ever be non-null, unless the value is set explicitly in TableMain... so if TableMain ever tries to raise the event itself, it won't call the same set of handlers as in TableSub. Basically, it's going to behave strangely.

The right approach is to provide a protected method in TableMain to allow the event to be raised by subclasses:

protected void OnUpdateFilter()
{
    Action handler = UpdateFilter;
    if (handler != null)
    {
        handler();
    }
}

Then make the event non-virtual, and remove the override in TableSub.

Note that your event signature doesn't match the normal convention for events - any reason for not using EventHandler?

Jon Skeet
This sounds like a big potential error source to me, because it is most likely not what you would expect when writing such code. Never had the need to think about that specific aspect before. Do you know why virtual events are syntactically allowed then anyway?
Thomas Weller
@John. Regarding --- "Note that your event signature doesn't match the normal convention for events - any reason for not using EventHandler?" I am just making use of the existing "Action" delegate, since in this case i dont need any arguments (object,eventargs) in the subscribers to this event. Is that (again) a bad programming practice? Is it a better idea to create a separate (empty) delegate for this case ?
Amby
@Thomas: There are ways you *could* use them sensibly - for example, you could log each subscription and then pass it up to the base implementation. Also you need to be able to have effectively abstract events to be able to put them in interfaces.
Jon Skeet
@Amby: It just means that event subscribers won't be able to use any general-purpose event handlers for your event. It's only a convention, and I admit I've never found it a *particularly* helpful one, but these days I tend to comply with it unless I've got a requirement which can't be satisfied that way. If you *do* want to stay away from the convention, then using `Action` is fine, and simpler than creating another delegate type.
Jon Skeet