Pattern for raising errors/warnings during parsing in a library

Question

I've got a library which has two input formats for the object model described by the library. I currently use an event subscription model for raising errors/warnings/verbose messages to the end user of the library. This hasn't proven to be the cleanest model, and I was wondering if there was a relevant design pattern or something similar found in the .Net Framework (when in Rome) to better handle this situation.

// Rough outline of the current code
public abstract class ModelReader : IDisposable
{
    public abstract Model Read();

    public event EventHandler<MessageAvailableEventArgs> MessageAvailable;

    protected virtual void RaiseError(string message)
    {
        var handler = this.MessageAvailable;
        if (handler != null)
        {
            handler(this, new MessageAvailaibleEventArgs(
                TraceEventType.Error, message);
        }
    }
}

Edit: some clarification. The Read routine will already fail fast on all Fatal errors using an exception. The messages are being logged to potentially multiple sources on the user's end, thus any pattern should avoid limiting the number of potential sources.

Answer 1

I can give you a real world example. The Html Agility Pack library is a parsing library. It simply defines a list of parsing errors on the the reader class. Extending your example, it would be something like:

    public abstract class ModelReader
    {
        private List<ParseError> _errors = new List<ParseError>();
        private bool _throwOnError;

        public ModelReader()
           :this(true)
        {
        }

        public ModelReader(bool throwOnError)
        {
           _throwOnError = throwOnError;
        }

        // use AddError in implementation when an error is detected
        public abstract Model Read();

        public virtual IEnumerable<ParseError> Errors
        {
           get {return _errors;}
        }

        protected virtual void AddError(ParseError error)
        {
           if (_throwOnError) // fail fast?
              throw new ParseException(error);

           _errors.Add(error);
        }
    }

    public class ParseError
    {
        public ParseError(...)
        {
        }

        public ParseErrorCode Code { get; private set; }
        public int Line { get; private set; }
        public int LinePosition { get; private set; }
        public string Reason { get; private set; }
        public string SourceText { get; private set; }
        public int StreamPosition { get; private set; }
    }

    public enum ParseErrorCode
    {
       InvalidSyntax,
       ClosingQuoteNotFound,
       ... whatever...
    }

    public class ParseException: Exception
    {
        ...
    }

And you still can add an event if the library caller wants on-the-fly events.

Answer 2

You seem to want a composable validator where your users can plug in their own logic to flag specific non fatal errors as either fatal, warning or informational messages. Throwing an exception does not fit the bill since you have left the method already if another part wanted to transform it into a warning but wanted to continue parsing. This sounds much like the two pass exception handling model in Windows for Structured Exception Handling. It basically goes like

1. Register as many exception handlers as you wish
2. When a problem is detected in pass all handlers are asked (no exeption throwing yet!) which one wants to handle the error. The first one which says yes will become the actual handler which decides what to do.
3. When a handler was found that can handle we come to pass 2 and call it. This time it is exception throwing time or not. It is entirely up to the handler.

The beauty of this two pass model is that during the first pass all handlers are asked but you still are able to continue parsing where you left. No state has been destroyed yet.

In C# you have of course much more freedom to turn this into a more flexible error transformation and reporting pipeline where you can transform for a strict reader e.g. all warnings to an error.

Which route you need to go depends on how your library is beeing used and how skilled the users of your library are and how many service requests you want to handle because some users make dumb errors. There is always a tradeoff between beeing as strict as possible (possibly too strict and you get a hard to use library) or too relaxed (nice parser but skips silently half of my input due to internal errors).

The Windows SDK Libraries are sometimes pretty hard to use because the engineers there optimize more towards less serivce calls. They throw a Win32 error code or HResult at you and you have to figure out which principle (memory alignment, buffer size, cross threading, ...) you have violated.

Answer 3

I think event subscription mode is OK. But you can consider interface instead. This might give you more flexibility. Something like this:

public interface IMessageHandler
{
    void HandleMessage(object sender, MessageAvailaibleEventArgs eventArgs);
}    

public abstract class ModelReader : IDisposable
{
    private readonly IMessageHandler handler; // Should be initialized somewhere, e.g. in constructor

    public abstract Model Read();

    public event EventHandler<MessageAvailableEventArgs> MessageAvailable;

    protected virtual void RaiseError(string message)
    {
        MessageAvailaibleEventArgs eventArgs =
            new MessageAvailaibleEventArgs(TraceEventType.Error, message);
        this.handler.HandleMessage(this, eventArgs);
    }
}

So now you can use any implementation of message handler, for exmaple your event subscription one:

public class EventMessageHandler : IMessageHandler
{
    public event EventHandler<MessageAvailaibleEventArgs> MessageAvailable;

    public void HandleMessage(object sender, MessageAvailaibleEventArgs eventArgs)
    {
        var handler = this.MessageAvailable;
        if (handler != null)
        {
            handler(this, new MessageAvailaibleEventArgs(
                TraceEventType.Error, message);
        }
    }
}

Answer 4

Your current approach isn't the cleanest model because it has two contradictory execution styles, pull (reading the input) and push (handling error callbacks), and that means both your reader and its clients require more state management to provide a coherent whole.

I recommend you avoid the XmlReader-like interface, and use the visitor pattern to push the input data and errors to the client application.

Answer 5

If there are error that stop the library from doing it's work I would use exception.

If this is strictly for tracing and instrumentation I would either go with your pattern or a TextWriter pattern, where the consumer would pass in a text writer and you would write your trace info to that, the only problem with that is you can only have one external subscriber. but it results in a slightly cleaner code.

 public TextWriter Log { get; set; }

 private void WriteToLog(string Message)
 {
    if (Log != null) Log.WriteLine(message);
 }

Answer 6

I know you mentioned that you may have several subscribers, so Event handlers and an Injected interface call are both good solutions as already mentioned above. For completeness I will also offer providing an optional Read() parameter as a Func. for example:

    void Read(Func<string, bool> WarningHandler)
    {
        bool cancel = false;
        if (HasAWarning)
            cancel = WarningHandler("Warning!");
    }

Then of course you could do whatever you wanted in the Func delegate, like divy the warning out to several sources. But when coupled with the Event model allows you to handle all Warnings in a general way (like a logger perhaps), and use the Func for individual specialized actions and control flow (if necessary).