[C#][Events] Is this correct? New to c#

Question

Been trying to learn c# and trying out events. Have tried it out, and got SOMETHING to function. When looking at my code it looks odd, because I have to create new objects for each subscriber, and then subscribe to the publisher. Do I need to create new object, and then subscribe to the publisher with that object?

Program.cs

namespace ConsoleApp1
{
    class Program
    {
        public static void Main()
        {
            ExternalClass potato = new ExternalClass();
            potato.Start();
            
        }
    }
}

Externalclass.cs

using System;

namespace ConsoleApp1
{
    class ExternalClass
    {
    
        //This is the event, which is the actual event you call to trigger all of the other method/function calls.
    
        public void Start()
        {
            string SubscribeMessage = "Subscribing...";
            string UnsubscribeMessage = "Unsubscribing";
            Apple potato = new Apple();
            Orange beet = new Orange();
            //adding a function to an event
            Console.WriteLine(SubscribeMessage);
            potato.MyEvent += potato.helloWorld;
            potato.MyEvent += beet.DisplayOrange;
            potato.OnEventSuccess();
            //unsubscribing from an event
            Console.WriteLine(UnsubscribeMessage);
            potato.MyEvent -= beet.DisplayOrange;
            potato.MyEvent -= potato.helloWorld;
            potato.OnEventSuccess();
        
        }
    }
}

Apple.cs

using System;

namespace ConsoleApp1
{
    class Apple
    {
        public event Action MyEvent;
    
        //This is the function that you wish to call when you call the event. All other function/method calls must have the same shape as the delegate
        public void helloWorld()
        {
            Console.WriteLine("Hello world!");
        }

        public void OnEventSuccess()
        {
            //myEvent?.Invoke();
            if (MyEvent != null)
            {
                MyEvent?.Invoke();
            }
            else
            {
                Console.WriteLine("Event is empty!");
            }
        }
    }
}

Orange.cs

using System;

namespace ConsoleApp1
{
    public class Orange
    {
        public void DisplayOrange()
        {
            Console.WriteLine("Orange is functioning");
        }
    }
}

Sample output:

Subscribing...
Hello world!
Orange is functioning
Unsubscribing
Event is empty!

Do you feel creating a publisher is weird or creating a subscriber is weird? — Louis Go, Jun 23 '20 at 01:12
@LouisGo maybe creating a subscriber in the start method feels weird? — Razazke, Jun 23 '20 at 01:17
You don't have to instantiate a brand new object for each event, you can register any method you want to the event, as long as the signatures match. — entropic, Jun 23 '20 at 01:18
@entropic are you saying multiple methods from one object to one event? or one object subscribe to multiple events? I am pretty sure the first is possible, and the second I know is possible. — Razazke, Jun 23 '20 at 01:22
I'm saying if you wanted, you don't need the `Orange` class at all, you can write the `DisplayOrange()` function in `ExternalClass` and register that to the event as well — entropic, Jun 23 '20 at 01:24
@Razazke, a short live object is not wrong. Nothing is always right nor always wrong. It depends on what you want to achieve. — Louis Go, Jun 23 '20 at 01:28
@LouisGo while nothing is always right nor always wrong, there are conventions to languages, and I like to follow some conventions(although some naming conventions are a bit much to me). — Razazke, Jun 23 '20 at 01:51
I'd say object lifetime is decided by usecase, but your naming is confusing. `Apple potato = new Apple();` seems like potato + apple, which is confusing while communicating with teammates. What if there is `Potato apple = new Potato();` Thing would get too complex to discuss with. Make naming meaningful is hard, and there is no "42" as a universal answer. — Louis Go, Jun 23 '20 at 01:57
For naming convention, you may follow [msdn](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/general-naming-conventions). — Louis Go, Jun 23 '20 at 02:03

score 0 · Answer 1 · answered Jun 23 '20 at 01:26

So far your code is okay.

For your question:

It looks like you feel creating new subscriber is not good. I'd say it is okay at the moment, unless you provide more context about your code.

Event is a way letting two objects communicate while decouple them, so the lifetime of subscribers or publishers doesn't matter. Just make sure that they live at the same time when event is fired, and unsubscribe them when unused.

However your code "might" have potential memory leak. Assume this potato is a member variable , it lives longer than beet.

Once any exception occurred before unsubscribing, it will never unsubscribe, and beet will live as long as potato.

    Apple potato = new Apple();
    public void Start()
    {
        string SubscribeMessage = "Subscribing...";
        string UnsubscribeMessage = "Unsubscribing";
        Orange beet = new Orange();
        //adding a function to an event
        Console.WriteLine(SubscribeMessage);
        potato.MyEvent += potato.helloWorld;
        potato.MyEvent += beet.DisplayOrange;
        potato.OnEventSuccess();

        // .... another operation
        // Something bad happened on the operation.
        // An exception is thrown then it never goes to Unsubscribe part.
        //unsubscribing from an event
        Console.WriteLine(UnsubscribeMessage);
        potato.MyEvent -= beet.DisplayOrange;
        potato.MyEvent -= potato.helloWorld;
        potato.OnEventSuccess();
    
    }

ref: How to avoid event handler memory leaks

[C#][Events] Is this correct? New to c#

1 Answers1