C# Implementation of Event

Question

I am learning about C# event implementation on classes.

I have example case:

There is a Sport and City classes inherit from Car class. Car class has base method call OnBuy that inherited by Sport and City classes. Inside OnBuy method, a event handler has beed assigned to Event Buy.

There is also a Service or Class called LicenseService, that generate license number every time has been bought.

I implemented event driven programming in this case. Here is my git sample:

https://github.com/adityosnrost/CSharpLearningEvent

Questions:

1. Is this the right way to use Event on C# ?

2. if this is a right one. Can I override OnBuy method into each childs ? and What can I do if override is available ?

3. What can I do to make it better from this sample ?

Thank you

class Program
{
    static void Main(string[] args)
    {
        Car car = new Car();
        Sport sport = new Sport();
        City city = new City();

        //car.Name();
        //sport.Name();
        //city.Name();

        //Console.ReadLine();

        LicenseService ls = new LicenseService();

        city.Buy += ls.GenerateLicense;

        city.OnBuy();

        Console.ReadLine();
    }
}

internal class Car
{
    internal virtual void Name()
    {
        Console.WriteLine("Car");
    }

    internal event EventHandler Buy;

    internal virtual void OnBuy()
    {
        EventHandler handler = Buy;
        if (null != handler)
        {
            handler(this, EventArgs.Empty);
        }
    }
}

internal class Sport: Car
{
    internal override void Name()
    {
        Console.WriteLine("Sport");
    }
}

internal class City: Car
{
    internal override void Name()
    {
        Console.WriteLine("City");
    }
}

internal class LicenseService
{
    internal void GenerateLicense(object sender, EventArgs args)
    {
        Random rnd = new Random();

        string carType = sender.GetType().ToString();

        string licenseNumber = "";

        for(int i = 0; i < 5; i++)
        {
            licenseNumber += rnd.Next(0, 9).ToString();
        }

        Console.WriteLine("{1} Car has been bought, this is the license number: {0}", licenseNumber, carType);
    } 
}

Answer 1

1) Is this the right way to use Event on C# ?

Not completely. The OnBuy should be a protected virtual method. That also excludes you calling it from the Main() method.

More common would be to call someCar.Buy() and then Buy() wil trigger OnBuy().

2) if this is a right one. Can I override OnBuy method into each childs ? and What can I do if override is available ?

Yes, you can override it See that as a more effective way of subscribing to yourself (which would be the alternative).

You can do anything needed when a specific type of Car is bought. But do alwas call base.OnBuy()

3) What can I do to make it better from this sample ?

The CreateLicense just doesn't sound like a good candidate for an event, it would be more of a business rule called by a CarDealer.

Following modern design rules, a Car would be a rather passive entity object (Anemic Domain Model).

Events would usually be used to tell other components "I have changed, do your thing", not to perform essential actions on self.

Answer 2

Nope.

Events should not be called by anyone other than the class that owns the event (or, if you know what you're doing, classes that inherit from the class that owns the event - a d even then, they should probably pay attention to the original implementation to avoid subtle issues)

Basically, a subscription to an event is a promise that, when a given thing happens, you'll call the functions you've fed to it from subscribers. The event itself is simple the code construct that allows you to make that subscription, without having to know or implement the intricacies of multicast function pointer invocation.

Otherwise, you're just calling a function and may as well not bother with an event.

Events are essentially intentional code injection - they allow you to make some other class execute arbitrary code that you wrote, when they do a thing.

Answer 3

If I was making this program, I would make following changes,

• Signature of Event Buy

first it was having two parameters Object and EventArgs where you only need Car in handler method (and also Random discussed below why).

• I would pass LicenseService in constructor of Child, and register (subscribe) Event in constructor only. that would be more cleaner way.
• Would make a string member CarName in parent class, so every child can use it anywhere they want.
• One more thing which i haven't done in this code, I would never name an event Buy, instead I would name it Bought .
• (This is specific to this scenario only) In your code, inside GenerateLicense() you are creating new object of Random every time. Thus If your two calls for that method are within no time , It will generate same Random number for both calls. Why? see this Question - or you can try below sample code by yourself. So I would pass Already created object of Random in GenerateLicense(). So Random will be common for every call of that method.

Sample code to explain Random number's behavior

        //as object of Random numbers are different,
        //they will generate same numbers
        Random r1 = new Random();
        for(int i = 0; i < 5; i++)
            Console.WriteLine(r1.Next(0, 9));
        Random r2 = new Random();
        for(int i = 0; i < 5; i++)
            Console.WriteLine(r2.Next(0, 9));

Update

• As suggested by Mrinal Kamboj (in comments below), we should not make Events exposed to external code. Adding his comment too in this answer

Two points, EventHandler Buy cannot be allowed to be directly accessed outside it shall be private, since anyone otherwise can set that to null and all subscription is gone. It needs an Event Accessor, so that event can be accessed using += and -= operators and there itself you make it thread safe for multiple subscribers, else there will be race condition, check a simple example

following is code,

your class structure:

internal delegate void EventHandler(Car car, Random rnd);
internal class Car
{
    internal string CarName;
    internal virtual void SetName()
    {
        this.CarName = "car";
    }

    //Edit : As Mrinal Kamboj suggested in comments below
    //here keeping event Buy as private will prevent it to be used from external code
    private event EventHandler Buy;
    //while having EventAccessros internal (or public) will expose the way to subscribe/unsubscribe it
    internal event EventHandler BuyAccessor
    {
        add 
        {
            lock (this)
            {
                Buy += value;
            }
        }
        remove
        {
            lock (this)
            {
                Buy -= value;
            }
        }
    }

    internal virtual void OnBuy(Random rnd)
    {
        if (Buy != null)
            Buy(this, rnd);
    }
}

internal class Sport: Car
{
    LicenseService m_ls;
    internal Sport(LicenseService ls)
    {
        this.m_ls = ls;
        this.BuyAccessor += ls.GenerateLicense;
        SetName();
    }

    internal override void SetName()
    {
        this.CarName = "Sport";
    }
}

internal class City: Car
{
    LicenseService m_ls;
    internal City(LicenseService ls)
    {
        this.m_ls = ls;
        this.BuyAccessor += ls.GenerateLicense;
        SetName();
    }
    internal override void SetName()
    {
        this.CarName = "City";
    }
}

LicenseService class

internal class LicenseService
{
    internal void GenerateLicense(Car sender, Random rnd)
    {
        string carName = sender.CarName;
        string licenseNumber = "";
        for(int i = 0; i < 5; i++)
        {
            licenseNumber += rnd.Next(0, 9).ToString();
        }
        Console.WriteLine("{1} Car has been bought, this is the license number: {0}", licenseNumber, carName);
    } 
}

and calling the flow

static void Main(string[] args)
{
    Random rnd = new Random();
    LicenseService ls = new LicenseService();
    Sport sport = new Sport(ls);
    City city = new City(ls);

    city.OnBuy(rnd);
    sport.OnBuy(rnd);

    Console.ReadLine();
}

Answer 4

Following would be my preferred design:

class Program
{
    static void Main(string[] args)
    {
        Car car = new Sport();
        car.BuyEvent += License.GenerateLicense;        
        car.OnBuy();
        car = new City();
        car.BuyEvent += License.GenerateLicense;
        car.OnBuy();
    }
}

internal abstract class Car
{
    internal abstract void Name();

    protected abstract event EventHandler Buy;

    public abstract event EventHandler BuyEvent;

    public abstract void OnBuy();   
}

internal class Sport : Car
{
    internal override void Name()
    {
        Console.WriteLine("Sport Car");
    }

    protected override event EventHandler Buy;

    public override event EventHandler BuyEvent
    {
        add
        {
            lock (this)
            {
                Buy += value;
            }
        }

        remove
        {
            lock (this)
            {
                Buy -= value;
            }
        }
    }

    public override void OnBuy()
    {
        if (Buy != null)
        {
            Buy(this, EventArgs.Empty);
        }
    }

}

internal class City : Car
{
    internal override void Name()
    {
        Console.WriteLine("City Car");
    }

    protected override event EventHandler Buy;

    public override event EventHandler BuyEvent
    {
        add
        {
            lock (this)
            {
                Buy += value;
            }
        }

        remove
        {
            lock (this)
            {
                Buy -= value;
            }
        }
    }

    public override void OnBuy()
    {
        if (Buy != null)
        {
            Buy(this, EventArgs.Empty);
        }
    }
}

internal static class License
{

    public static void GenerateLicense(object sender, EventArgs args)
    {
        Random rnd = new Random();

        string carType = sender.GetType().Name;

        string licenseNumber = "";

        for (int i = 0; i < 5; i++)
        {
            licenseNumber += rnd.Next(0, 9).ToString();
        }

        Console.WriteLine($"{carType} Car has been bought, this is the license number: {licenseNumber}");
    }

}

Important points:

1. Make the Car base class abstract, it shall only be derived and used like City / Sport Car
2. Add event add / remove accessor in each of the child class for custom processing of the event. Make the accessor thread safe for shared client access
3. Make License - GenerateLicense as either static, as there's no state / data in the class or integrate them too with the City / Sport class implementation by having another base abstract method
4. Abstract class Car shall be injected with Sport \ City objects at the runtime and shall be used for event subscription via base class Car object

Answer 5

It's a bit of a long answer, but I will address your example first and then move on to some general tips. Bare in mind all code is pseudo code, it will not compile without syntax adjustments.

First of all, your logical structure does not make sense, which is why it might be hard for you to pinpoint if this is correct or not.

For instance in real world, you do not address a car to buy it, you address a shop or a service that sells them. You only address a car to drive, or use other functionality that it offers. The car does not assign itself a license. Finally, a purchase is a process that is generally linear (and can be expressed by a method, without triggers) if you take a basic sellers/buyer example. So when you call shop.BuyCar( sportsCar ) all of the purchase logic can be called from the buy method.

Class Shop{ 

    public Car BuyCar( carWithoutLicense ){
        //Purchase logic
        LicenseService.AssignLicense( carWithoutLicense ).
        return carWithoutLicense.
    }
}
//A person would call this method, no the car

A better example of correctly utilized event would be one of the alert lights on the front panel of a car, because it is there to notify the driver of something he/she might want to react to. For instance: a check engine light.

class Car {
   Bool CheckEngingLightIsOn = false;
   public void Ride( ... ){
    if( enginge.faultDetected == true ){
       TurnOnCheckEngineAlert( );
    }
   }
   public void TurnOnCheckEngineAlert( ){
      CheckEngingLightIsOn = true;
      if( CheckEngineLightSwitch != null ){
         CheckEngineLightSwitch.Invoke( ... )
      }
   }
}

class Driver {
   public Driver( Car car ){
      this.car = car;
      if( driverState != Drunk ){
       car.CheckEngineLightSwitch = TakeAction;
      }
   }
   public Drive( ){
      car.Ride( );
   }
   public void TakeAction( Car car, EventArgs e ){
      //get out, open the hood, check the engine...
      if( car.CheckEngingLightIsOn == true ){ "Light turned on
        //Check Engine
      }else{
        //continue driving
      }
   }
}

Without getting too deep into abstraction, notice the chain of events:

• Driver drives a car and does not worry about other things (such as that check engine light) until they happen.
• If Car detects a fault, it turns on the check engine light and there are event handlers on that event (subscribers) the car triggers the event.
• The event fires, but the driver has to be subscribed to it to notice the change.
• ONLY if the driver is subscribed to that event (in this case, if not drunk) he will take action, based on that event.

This example is fundamentally different to your example, because:

• While driving the car, the driver doesn't need to pay attention to check engine light all the time (even though, he can check it).
• It IS the standard process of a car to check engine state and represent it on the engine light.
• Driver and the Car both affect each others further actions and this logic is inefficient if express linearly.

In other words, Buying process is set in stone (paying, license, goods transfer) and skip essential steps cannot be skipped. The process of a car moving itself is not set in stone, because neither the car nor the driver knows what will happen in journey. I.E. Driver might drive to destination without stopping (if no fault develops or if he doesn't notice the light) or the car might force the driver to stop when he notices the check engine light go on (essentially, they both control each other in a sense).

---

Some general tips about your use-case

In your example You have made a somewhat complex use-case (with incorrectly placed logic), which it will run, but will be structurally incorrect (and bad logical structure tends to lead to human mistakes when designing further logic).

First thing you should look at is what logic is relevant to each object. Does an event/method in your object represent something that your object does (i.e. functionality that an object performs itself) or something that affects your object but the object itself does not do anything in process? For instance: a car does "riding" on it's own (even if start of this process and all of its parameters, such as speed or direction are controlled by the driver); assigning license to a car happens completely outside of the car structure and the car only get an attribute changed (license) in process.

This distinction is important because only logic performed by your object is relevant to that object, and by extension, any logic that is performed by another object and only affects your object is irrelevant belongs somewhere else. So Buy definitely does not belong in the car and Ride (the process of moving) belongs to the car.

Secondly, your naming will go a long way to help you understand this topic. Methods represent actions and should be named as such (Shop.BuyCar, Car.Ride, Driver.Drive ), events represent reaction triggers ( Car.CheckEngineLightSwitch) and event handlers represent reactions to an action (a reaction is still an action, so no special naming is needed, but you can name to to make a distinction between an action and a reaction).

Answer 6

I recommend you to inject licence service to car, and call generate licence function when buy is called,

using System;

namespace ConsoleApp1.Test
{

    class Program
    {
        static void Maintest(string[] args)
        {
            ILicenseService licenseService = new LicenseService();

            Sport sport = new Sport(licenseService);
            City city = new City(licenseService);

            //car.Name();
            //sport.Name();
            //city.Name();

            //Console.ReadLine();            

            city.OnBuy();

            Console.ReadLine();
        }
    }

    internal abstract class Car
    {
        protected readonly ILicenseService licenseService;

        public Car(ILicenseService _licenseService)
        {
            licenseService = _licenseService;
        }

        internal virtual void Name()
        {
            Console.WriteLine("Car");
        }

        internal event EventHandler Buy;

        internal virtual void OnBuy()
        {
            // TODO
        }
    }

    internal class Sport : Car
    {
        public Sport(ILicenseService _licenseService) : base(_licenseService)
        {

        }

        internal override void Name()
        {
            Console.WriteLine("Sport");
        }

        internal override void OnBuy()
        {
            licenseService.GenerateLicense(new object());
        }
    }

    internal class City : Car
    {
        public City(ILicenseService _licenseService) : base(_licenseService)
        {

        }

        internal override void Name()
        {
            Console.WriteLine("City");
        }

        internal override void OnBuy()
        {
            licenseService.GenerateLicense(new object());
        }
    }

    internal interface ILicenseService
    {
        void GenerateLicense(object param);
    }

    internal class LicenseService : ILicenseService
    {
        public void GenerateLicense(object param)
        {
            // do your stuff
        }
    }

}