Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I plan to make use of this interface in a plug-in architecture.

/// <summary>
/// Generic interface allowing you to react to an event.
/// You can block the event or just use it for notification.
/// </summary>
/// <typeparam name="T"></typeparam>
public interface IEventObserver<in T> where T : class
{
    /// <summary>
    /// Throw an exception in here if you want to prevent the event firing.
    /// </summary>
    /// <param name="o"></param>
    void CanFireEvent(T o);

    /// <summary>
    /// This is only called if no handler threw an event in the CanFire.
    /// </summary>
    /// <param name="o"></param>
    void OnBeforeEvent(T o);

    /// <summary>
    /// This is called after the event has happened.
    /// </summary>
    /// <param name="o"></param>
    void OnAfterEvent(T o);
}

Example usage:

public interface ISendMessageObserver : IEventObserver<SendMessageEventArgs>
{
}

A plug-in can then implement ISendMessageObserver and prevent message sending by throwing an exception in the CanFireEvent method. However, I know that exceptions should not be used for normal program flow control, yet it is normal and expected for plugins here to stop the flow. I went for this because I'd like the plugin to provide a reason for blocking the event that can be logged or presented to the user.

I could change CanFireEvent to:

bool CanFireEvent(T o, ref message);

or:

SomeTypeContainingAbortReason CanFireEvent(T o);

But both of those don't smell ideal either.

So what do people think, exceptions for flow control OK in this instance?

Edit

I've separated the CanFire into it's own interface and renamed it as PreviewEvent.

/// <summary>
/// Generic interface allowing you to react to and block an event.
/// </summary>
/// <typeparam name="TArgs"></typeparam>
/// <typeparam name="TBlock"></typeparam>
public interface IEventPreviewer<in TArgs, out TBlock>
    where TArgs : class
    where TBlock : class, IEventBlock
{
    /// <summary>
    /// Return an IEventBlock to prevent the event from firing.
    /// </summary>
    /// <param name="args"></param>
    TBlock PreviewEvent(TArgs args);
}

public interface IEventBlock
{
    string BlockReason { get; }
}
share|improve this question
    
Your second alternative implies that it's still an error that you're dealing with: "SomeTypeContainingErrorInfo CanFireEvent(T o);" If they are errors/exceptions, I would say that it's fine. Otherwise, no, go with your second alternative, as I simply don't see what smells about it. –  Eoin Carroll May 28 '13 at 8:35
    
@EoinCarroll good point, I have renamed that SomeTypeContainingAbortReason. I guess that's part of the problem, I don't know whether all reasons for blocking events will be errors because that's up to the plugin implementor. For the most extream example I might write a plugin that blocks all messages, it's not an error and it's not exceptional, that's just's the plugins job. –  weston May 28 '13 at 8:41
    
@EoinCarroll yes I think I will go with second one. I'll make it generic too, so ISendMessageObserver can have a different return type to other IEventObservers. –  weston May 28 '13 at 8:43
add comment

1 Answer

up vote 4 down vote accepted

No, exceptions are never the right tool for normal flow control. Exceptions should only be used to indicate that a function/method can't fulfil its contract due to external reasons.

In the case of your CanFireEvent method, the function name clearly suggests a yes/no answer and thus a boolean return value, but the second alternative (where the 'No' gets elaborated with a fixed reason also fits.

If I were to have such a function in my event system, I would definitely not allow a free-format reason to be given. What use is it to see the reason "Because I can."

Come to that, allowing plug-ins to prevent messages from being sent makes for an extremely brittle system. You only need one plug-in that consistently blocks all messages to break the whole system. You could have a bool CanHandleEvent(T) method, with which the plug-in can indicate it it can do something useful with the event, but having a CanFireEvent is just too broad. And most event systems don't even have the CanHandleEvent, they just expect you to be able to handle events at all times (where handling can be that you ignore it).

Update

Based on the comments, I see that the described event mechanism isn't as generic as I thought and that blocking events does not have the meaning "this event can't be handled now", but rather "the proposed action is not valid according to the business rules". That changes the last part of my answer a bit.

If business rules can affect the validity of an action, and this can't be easily determined beforehand, then I would introduce an interface in the system along the lines of

public interface IActionValidator<in T> // or IEventValidator if you prefer
where T : class
{
    bool IsActionValid(T o, ref message);
}

I have separated this from the interface for actually handling the action, as I can imagine that the two are implemented in separate classes/components.

share|improve this answer
    
Thanks. Messages was just an example of course, this is a stock system, and different customers have different rules on things and the example I have in mind is moving of stock around a warehouse. Most clients have no restrictions, but some have restrictions on the products dates, or whatever, the feedback has to come back to the user, something like "Can't move product, it's too old". How else can I achieve this with the core of the system not knowing too much about the rejection reasons? But I think I'm definitely going to segregate the interface in to IEventObserver and IEventPreventor –  weston May 28 '13 at 9:14
    
I've put an edit on the question with how my new smaller separate interface looks (IEventPreviewer). It's now separate from the observer pattern so as not to cloud things. I'd apreciate advice on better ways to do achieve this though. –  weston May 28 '13 at 9:31
    
@weston: see my update –  Bart van Ingen Schenau May 28 '13 at 10:06
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.