C# raising events

Question

I've been working a lot with C# recently, and I've noticed that most code that raises events in my company's code is done like this:

EventHandler handler = Initialized;

if (handler != null)
{
    handler(this, new EventArgs());
}

I really don't understand why, instead, you can't do just do that:

if (Initialized != null)
{
    Initialized(this, new EventArgs());
}

EDIT:

Some food for thought, I tried doing a few tests on this:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            Test t = new Test(true);

            while(true)
            {
                t.Ev += new EventHandler(t_Ev);
                t.Ev -= new EventHandler(t_Ev);
            }
        }

        static void t_Ev(object sender, EventArgs e)
        {
        }
    }

    public class Test
    {
        private readonly bool m_safe;

        public Test(bool safe)
        {
            m_safe = safe;

            Thread t = new Thread(Go);
           t.Start();
        }

        private void Go()
        {
            while (true)
            {
                if(m_safe)
                {
                    RaiseSafe();
                }
                else
                {
                    RaiseUnsafe();
                }
            }
        }

        public event EventHandler Ev;

        public void RaiseUnsafe()
        {
            if(Ev != null)
            {
                Ev(this, EventArgs.Empty);
            }
        }

        public void RaiseSafe()
        {
            EventHandler del = Ev;

            if (del != null)
            {
                del(this, EventArgs.Empty);
            }
        }
    }
}

The Unsafe version makes the program crash.

I guess you need to ask the people who wrote the original code. — David Heffernan, Feb 17 '12 at 14:48
See http://blogs.msdn.com/b/ericlippert/archive/2009/04/29/events-and-races.aspx — Eric Lippert, Feb 17 '12 at 16:02

score 9 · Accepted Answer · edited May 23 '17 at 12:23

9

The first version is an attempt to make the event thread-safe.

See C# Events and Thread Safety

Actually, as said in the discussion, it doesn't make the event thread safe. Thus I would use the second version that is shorter instead.

EDIT: event thread-safety is really hard to implement. Look at what it might look... If you're not actually dealing with multiple threads that register/unregister events, you shouldn't waste time with thread-safety.

edited May 23 '17 at 12:23

Community

1
1

answered Feb 17 '12 at 14:49

ken2k

48,145
10
116
176

2

heh... was trying for that, but you beat me to it :) – Richard B Feb 17 '12 at 14:50

VoodooChild · Answer 2 · 2012-02-17T15:04:19.607

0

The second version is not thread-safe.

if (Initialized != null)
{
    Initialized(this, new EventArgs());
}

If the last handler unsubscribes after the if (Initialized != null) then you will end up with null ref exception.

edited Feb 17 '12 at 15:04

answered Feb 17 '12 at 14:51

VoodooChild

9,776
8
66
99

3

Actually neither version is threadsafe, depending on your definition of threadsafe. Sure, you can avoid the null dereference, but the first version does not avoid *potentially invoking a handler that has been removed on another thread.* – Eric Lippert Feb 17 '12 at 16:01

C# raising events

2 Answers2