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

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();
}