4

I am reverse-engineering a legacy application that is no longer supported and has stopped working so I can create a new one to perform the same function. I have this class that uses the following code pattern quite a lot.

        public event myEventHandler myEvent
        {
            [MethodImpl(MethodImplOptions.Synchronized)]
            add
            {
                myEvent = (myEventHandler)Delegate.Combine(myEvent, value);
            }
            [MethodImpl(MethodImplOptions.Synchronized)]
            remove
            {
                myEvent = (myEventHandler)Delegate.Remove(myEvent, value);
            }
        }

However, Visual Studio returns the error: "myEvent can only appear on the left-hand side of += or -="

Does the following code to solve this error? Is there any difference between delegate.combine and using += ?

        public event myEventHandler myEvent
        {
            [MethodImpl(MethodImplOptions.Synchronized)]
            add
            {
                myEvent += value;
            }
            [MethodImpl(MethodImplOptions.Synchronized)]
            remove
            {
                myEvent -= value;
            }
        }
StayOnTarget
  • 11,743
  • 10
  • 52
  • 81
HuwD
  • 1,800
  • 4
  • 27
  • 58
  • Would the issue here not be `myEvent = (myEventHandler)Delegate.Combine(myEvent, value);`? Calling Delegate.Combine would suggest that they are combined inside it and it is of a `void` return type. try just `Delegate.Combine(myEvent, value);` – Alfie Goodacre Nov 25 '16 at 10:35
  • @Alfie this wouldn't work. The MSDN says: Returns a new delegate with an invocation list that concatenates the invocation lists of the delegates in the delegates array. Returns null if delegates is null, if delegates contains zero elements, or if every entry in delegates is null. – Rabban Nov 25 '16 at 10:38
  • @Rabban thanks, didn't check the documentation and was just throwing things out there :) – Alfie Goodacre Nov 25 '16 at 10:39
  • 2
    @HuwD Yes, you can use `+=` to replace `Delegate.Combine(myEvent, value)` – Rabban Nov 25 '16 at 10:42
  • 2
    just remove that block. you simply want to subscribe to event. there is no need for `add` and `remove`. simply type `public event myEventHandler myEvent;` and you are good to go – M.kazem Akhgary Nov 25 '16 at 10:42
  • 4
    This block is functionally equivalent (though with a slightly different and most likely less efficient implementation) than what the C# compiler itself generates when you declare an `event`, including thread safety. This code is actively harmful by wrapping this code again in its own block. You can simply reduce the whole thing to `public event myEventHandler myEvent;` with no loss in function or safety. – Jeroen Mostert Nov 25 '16 at 10:50
  • 2
    Most likely that code is just produced by your decompiler (with which you reverse engineer). Original code was just declaring simple event (public event myEventHandler myEvent). – Evk Nov 25 '16 at 11:10
  • 1
    Thanks for some great responses. Will try simply subscribing to the event as suggested @M.kazem Akhgary but is good to know I can fall back on += & -= if required. – HuwD Nov 25 '16 at 11:34
  • Possible duplicate of [-event- can only appear on the left hand side of += or -=](https://stackoverflow.com/questions/4496799/event-can-only-appear-on-the-left-hand-side-of-or) – StayOnTarget Feb 20 '19 at 17:25

1 Answers1

6

You are using property syntax for the event. That means that you are not defining an actual delegate. You are just creating methods that will add and remove delegates. In specific, you don't have a delegate variable myEvent, you are just creating add and remove methods for it. That means that your code...

        add
        {
            myEvent += value;
        }

...would be an infinite recursion, if it were allowed. Why?, because the operator += translates to an actuall call to the add method of the event. And since you are already in the add method, you are practically saying: add the event by adding the event. The same applies for -= and remove.

The solution is to create a backer field for your event that you can assign the value to:

    private myEventHandler _myEvent = null;

    public event myEventHandler myEvent
    {
        [MethodImpl(MethodImplOptions.Synchronized)]
        add
        {
            _myEvent += value;
        }
        [MethodImpl(MethodImplOptions.Synchronized)]
        remove
        {
            _myEvent -= value;
        }
    }

Note that the event keyword is missing on _myEvent, which is on purpose, since it is not an event, but a field that stores the event handlers.

In your case though, you won't need property syntax at all, since all you do is to pass through the handler (given your MethodImplAttribute attached to the methods is not needed):

public event myEventHandler myEvent;

And don't use Delegate.Combine. It's dirty, unnecessary and won't solve your problem.

Sefe
  • 13,731
  • 5
  • 42
  • 55