views:

3203

answers:

6

Hi all,

I have a custom collection that I am adding a ValidateItem event to. This ValidateItem event will be called whenever an item is added to or updated in the custom collection.

I want to allow derived classes to be able to subscribe to the event and determine their own logic for whether or not an item is "valid" and potentially prohibit it being added to the collection if it is "invalid".

But I am trying to figure out how to let the caller to the event know what is going on and how to pass the information about what is going on back.

My custom eventargs inherit from CancelEventArgs so I have the ability to pass the Cancel bit back to the caller using that. But I have never seen any cases where error information (error codes, messages, etc) gets passed back in this manner, so I'm wondering if this might not be the best approach.

Should I just add any error data that I wish to pass back to my Custom eventargs class, are there good reasons for or against this? or are there other, better ways to accomplish this?

Here is my eventargs class:

public delegate void ItemValidationEventHandler(object sender, ItemValidationEventArgs e);

public class ItemValidationEventArgs : CancelEventArgs
{
    public ItemValidationEventArgs(object item, ObjectAction state, EventArgs e)
    {
        Item = item;
        State = state;
        EventArgs = e;
    }

    public ItemValidationEventArgs(object item, ObjectAction state) : this(item, state, new EventArgs())
    {
    }

    public ItemValidationEventArgs() : this(null, ObjectAction.None, new EventArgs())
    {
    }

    // is there a better way to pass this info?
    public string ErrorMessage {get; set;}
    public int ErrorNumber {get;set;}

    public object Item { get; private set; }
    public ObjectAction State { get; private set; }

    public EventArgs EventArgs { get; private set; }
}

UPDATE: I suppose another other option would be to use something like this:

virtual bool Validate(object item, ObjectAction action, out string errorMessage)

method in the derived classes. Though I tend to prefer avoiding out parameters...

If anyone has any ideas on the pros and cons of each approach I'd love to hear em!

Thanks, Max

A: 

Well, if you create your own custom validation events, along with custom validation event args, I would assume you'd prefer to pass back status/error codes rather than throwing exceptions if something doesn't validate.

In this case, if you want to have validation that will not throw exceptions, then yes, I'd add those fields you need to your custom event args - they're custom already, so there's no reason not to extend them to suit your needs :-)

Marc

marc_s
A: 

A couple of quick thoughts:

I would make the properties readonly to conform to the "normal" pattern of eventargs classes.

Perhaps the properties relating to error information should be wrapped together into some ErrorInformation class (that would make it a bit easier to pass that information around to other methods).

A question: what do you use the EventArgs property for?

Fredrik Mörk
I think readonly wouldn't work in this case. If you look at CancelEventArgs, the bool Cancel is not readonly... and that is used for passing information back to the caller, so really all I am doing is extending that a little. Something just strikes me as "funny" about this approach, I can't really put my finger on it.I'm using the event args to pass an object (item) out to the validate method, the validate method does whatever it wants... in the simplest example it checks that the item doesn't already exist in the collection, and then returns Cancel bool and potentially any error messages.
Max Schilling
Of course; I was thinking in the wrong direction... Another thing struck my mind; what happens if there are two different listeners to the validation event where one sets Cancel = true, and the other one sets Cancel = false? I get the same feeling as you; there is something bad lurking under the surface here ;o)
Fredrik Mörk
A: 

Custom event args are specifically used to pass information to and from the event handlers, so yes, they're a good place to put them.

The other option would be to throw an exception - this is fairly heavy-handed, but should stop other event handlers from being executed for the event.

Jason Williams
A: 

EventArgs and CancelEventArgs don't have any members to carry information to the subscriber. So, you would normally inherit from one of these classes and add one or more members to carry info. You can also make your class generic, so that you don't have to make a new EventArgs class whenever you have different data to send.

Finally, instead of making your own ItemValidationEventHandler delegate type, you can use EventHandler<T>, where T is the type of your EventArgs parameter.

Also, you don't need the EventArgs = e member.

mbeckish
+1  A: 

I don't think what you are describing really fits the observer, observable pattern that you would normally see with events.

Since these are derived classes I would probably look to virtualizing the validation method of the objects and having the children provide a specific validation routine.

Dan Blair
I think that is part of what is bothering me about this approach. But then doesn't the CancelEventArgs having a modifiable Cancel property already violate the pattern? So then, hypothetically, what is wrong with adding a little extra data?
Max Schilling
The cancel event arg tells other events down the line that the event was cancelled. It's a way for other observers to know not to do something. One of the problems with events is that all the subscribers are going to get called in some non deterministic order, and that's not exactly what you want to happen here, you only want one objects validate called.
Dan Blair
+3  A: 

Using events for this is probably not the best design approach.

Since it is an inheriting class that would be overriding this behavior, the method should be marked as protected and virtual:

protected virtual bool Validate(object item);

I too do not like using out on parameters, so following your initial instincts to use EventArgs, you should probably create a class to encapsulate the results of your validation.

Example:

class ValidationResult
{
     public string ResultMessage{get;set;}
     public bool IsValid {get;set;}
}

Your method would then be:

protected virtual ValidationResult Validate(object item)
{
   ValidationResult result = new ValidationResult();

   // validate and set results values accordingly

   return result;
}

The pros and cons of using this over events are that events are intended to be used when you want to publish an action or information to multiple subscribers. The subscribers are classes that you know nothing about. You do not care who they are or what they do. They should never really pass information back to the notifying class either. They should just process the event information given to them.

In your instance, your inherited class is your only subscriber. On top of that you want to be able pass useful information back to the parent class. Inheritance fits this desired behavior much better, and also allows you to easily implement different types of validation class. With events, you would have to keep typing code to attach event handlers over and over again (very ugly IMO).

jnosek
Quite well said +1
Dan Blair