Search code examples
c#eventsdelegatescustom-events

Error when raising custom event


I have a class that will write a log. The class needs to raise an event (under specific circumstances not indicated below), that will be comsumed by a class to react on it. I have the code below but as soon as I try to raise the event, I get an error on the line as indicated, that

Object reference not set to an instance of an object

Any idea what I'm missing?

//1. Class where event is registered
    public class LogEvent
    {
        public delegate void WriteLogEventHandler(object Sender, WriteLogEventArgs e);
        public event WriteLogEventHandler WriteLog;

        public class WriteLogEventArgs : EventArgs
        {
            public string Message { get; set; }

            public WriteLogEventArgs(string message) : base()
            {
                Message = message;
            }
        }

        //Raise the event.
        internal void OnWriteLog(WriteLogEventArgs e)
        {
             WriteLog(this, e);    //Error here.  Seems like WriteLog is null
        }

//2. Class where event is raised.
public class Logs
{
    public static void WriteLog(string message)
    {
        LogEvent.WriteLogEventArgs args = new LogEvent.WriteLogEventArgs(message);
        new LogEvent().OnWriteLog(args);
    }
}

//3. Class where event should be consumed
public class MyClass()
{
    private LogEvent _logEvent;
    public MyClass()
        {
            //Subscribe to event:
            _logEvent = new LogEvent();
            _logEvent.WriteLog += (sender, args) => { DoSomething(args.Message); };
        }

   public void DoSomething(string message)
   { ... }
}

Solution

  • Two issues:

    • You're raising the event whether or not anyone has subscribed to it. Don't do that - you will get a NullReferenceException if you call WriteLog(this, e) when WriteLog is null. In C# 6 it's easy to avoid this:

      WriteLog?.Invoke(this, e);
      
    • You're subscribing to the event on a different LogEvent instance than the one which is raising the event. This is more of a design issue than anything else - it makes no sense for an individual log event to have a list of subscribers. Instead, you should have a Logger or similar which has the subscribers (via an event), then each LogEvent is passed to those subscribers. You'd create one Logger, subscribe to it, then call WriteLog on the same instance.