Shopping cart using events and delegates

Question

I am trying to learn events and delegates in C#. To do that I am trying to create a hypothetical console project where when some user submits the orders in his/her shopping cart, I need the Billing department and mailing department to handle that event. Can someone please critique this code for doing the same?

using System;
using System.Collections.Generic;

namespace LearnEvents
{
    class Program
    { 
        static void Main(string[] args)
        {
            var shoppingCart = new ShoppingCart(10);//shopping cart for user number 10
            var bd = new BillingDepartment(shoppingCart);
            var md = new MailingDepartment(shoppingCart);
            shoppingCart.Add(75458);
            shoppingCart.Add(54693);
            shoppingCart.Add(52145);
            shoppingCart.Submit();

        } 

    }

    public class ShoppingCart
    {
        private int _userId ;
        private List<int> _orders=new List<int>();
        public delegate void OrderSubmitted(OrderDetails orderDetails);
        public  event OrderSubmitted OrderSubmittedEvent;


        public ShoppingCart(int userId)
        {
            _userId = userId;
        }
        public void Add(int itemNumber)
        {
            _orders.Add(itemNumber);
        }

        public void Submit()
        {
            OrderSubmittedEvent.Invoke(new OrderDetails{ItemCodes = _orders,UserId = _userId});
        }

    }

    public class OrderDetails
    {
        public List<int> ItemCodes { get; set; }
        public int UserId { get; set; } 
    }


    public class BillingDepartment
    {
        public  BillingDepartment(ShoppingCart sc)
        {
             sc.OrderSubmittedEvent+=OrderSubmittedHandler;
        }

        public void OrderSubmittedHandler(OrderDetails orderDetails)
        {
          foreach (var item in orderDetails.ItemCodes)
            {
                Console.WriteLine("Billing user "+orderDetails.UserId+" for the order "+item.ToString());
            }
        }

    }

    public class MailingDepartment
    {

        public MailingDepartment(ShoppingCart sc)
        {
            sc.OrderSubmittedEvent += OrderSubmittedHandler;
        }
        public void OrderSubmittedHandler(OrderDetails orderDetails)
        {
            foreach (var item in orderDetails.ItemCodes)
            {
                Console.WriteLine("Mailing user " + orderDetails.UserId + " the order " + item.ToString());
            }
        }

    }


}

I would also like to know, why do I need an event? Why can't I just use multicast delegates instead as shown below?

using System;
using System.Collections.Generic;

namespace LearnEvents
{
    class Program
    { 
        static void Main(string[] args)
        {
            var shoppingCart = new ShoppingCart(10);//shopping cart for user number 10
            var bd = new BillingDepartment(shoppingCart);
            var md = new MailingDepartment(shoppingCart);
            shoppingCart.Add(75458);
            shoppingCart.Add(54693);
            shoppingCart.Add(52145);
            shoppingCart.Submit();

        } 

    }

    public class ShoppingCart
    {
        private int _userId ;
        private List<int> _orders=new List<int>();
        public delegate void OrderSubmitted(OrderDetails orderDetails);

        public OrderSubmitted os;


        public ShoppingCart(int userId)
        {
            _userId = userId;
        }
        public void Add(int itemNumber)
        {
            _orders.Add(itemNumber);
        }

        public void Submit()
        {
            os.Invoke(new OrderDetails { ItemCodes = _orders, UserId = _userId });
        }

    }

    public class OrderDetails
    {
        public List<int> ItemCodes { get; set; }
        public int UserId { get; set; } 
    }


    public class BillingDepartment
    {
        public  BillingDepartment(ShoppingCart sc)
        {
            sc.os+=OrderSubmittedHandler;  
        }

        public void OrderSubmittedHandler(OrderDetails orderDetails)
        {
          foreach (var item in orderDetails.ItemCodes)
            {
                Console.WriteLine("Billing user "+orderDetails.UserId+" for the order "+item.ToString());
            }
        }

    }

    public class MailingDepartment
    {

        public MailingDepartment(ShoppingCart sc)
        {
            sc.os += OrderSubmittedHandler; 
        }
        public void OrderSubmittedHandler(OrderDetails orderDetails)
        {
            foreach (var item in orderDetails.ItemCodes)
            {
                Console.WriteLine("Mailing user " + orderDetails.UserId + " the order " + item.ToString());
            }
        }

    }


}

Answer 1

The standard signature for an event-handler delegate is

void HandlerName(object sender, HandlerArgs args)

Where HandlerArgs inherits from System.EventArgs

So to follow this convention, you should create a new class:

 class OrderSubmittedEventArgs : EventArgs
 {
       OrderDetails { get; set; }
 }

And then you could theoretically define the event as:

 //... 
 public delegate void OrderSubmittedHandler(object sender, OrderSubmittedEventArgs eventArgs);
 public  event OrderSubmittedHandler OrderSubmitted;

But that's more code than we need, now that we're following the convention. .NET provides a helper class:

 //
 public event EventHandler<OrderSubmittedEventArgs> OrderSubmitted;

---

You also ask another question - why use an event rather than a multicast delegate?

You can think of an event as a multicast delegate, but with restrictions.

• Only the object that owns the event can invoke it.
• Another object can add or remove a listener, but cannot modify it in any other way

For example:

class TestClass
{
    public EventHandler ThisDelegate;
    public event EventHandler ThisEvent;

    private void TryThis()
    {
        if (this.ThisEvent != null)
        {
            // I can fire my own event
            this.ThisEvent(this, EventArgs.Empty);
        }

        // I can clear my own event
        this.ThisEvent = null;
    }
}


class OtherClass
{
    void Test()
    {
        var test = new TestClass();
        // I can invoke test's delegate!
        test.ThisDelegate(this, EventArgs.Empty);
        // I can clear test's delegate!
        test.ThisDelegate = null;

        // But I can't do that to its event
        test.ThisEvent(this, EventArgs.Empty); // Compiler error
        test.ThisEvent = null;  // Compiler error
    }
}

So, by making the member an Event rather than a Multicast Delegate, you give it extra semantic meaning that is enforced by the compiler. You do this for similar reasons that you mark members as private.

Answer 2

I think you need another entity to delegate events, it makes no sense to pass shopping cart to your "departments" constructors.

class Shop
{
    public ShoppingCart GetCart(int id)
    {
        var cart = new ShoppingCart(id);
        shoppingCart.OrderSubmittedEvent += OnOrderSubmitted;
        return cart;
    }

    private BillingDepartment _billingDepartment = new BillingDepartment(shoppingCart);
    private MailingDepartment _mailingDepartment = new MailingDepartment(shoppingCart);

    private void OnOrderSubmitted(OrderDetails orderDetails)
    {
        _billingDepartment.Bill(orderDetails);
        _mailingDepartment.Submit(orderDetails);

        //notice unsubsription, its a good style to do that 
        //even if you dont really need to
        //your current event signature does not allow access to shopping cart tho.
        //it probably should

        //shoppingCart.OrderSubmittedEvent -= OnOrderSubmitted;
    }
}

Usage:

var shop = new Shop();
var cart = shop.GetCart(10);
cart.Add(75458);
cart.Add(54693);
cart.Add(52145);
cart.Submit();

Also this:

public void Submit()
{
    OrderSubmittedEvent.Invoke(new OrderDetails { ItemCodes = _orders, UserId = _userId });
}

is bad code. The proper way do invoke events is:

public void Submit()
{
    //copy to local variable to avoid racing i multithreading environment
    var ev = OrderSubmittedEvent;
    //check for null
    if (ev != null)
    {
        //only then invoke
        ev(new OrderDetails { ItemCodes = _orders, UserId = _userId });
    }
}

Andrew also makes valid point about common conventions when it comes to events, check his answer. In my inner modules, i would probably break it and use Action<T> as event type though, but in code availible to others you should use EventHandler<T>.