Are the over-usage of delegates a bad thing?

Question

Good day.

I have been told that a good practice for writing methods is to make the method only responsible for doing one thing. It made sense to me now that I look back upon it in retrospect. There are a couple of reasons I like coding in such a way - to me they are:

1. Code is Manageable
2. Code is more human readable
3. Better understanding of what the method does is maintained

But here comes the issue which concerns me. Sometimes performing a task requires more than 1 action to be performed. A lot of developers' code which I look at every day does 7 to 8 things per method, where it could have been broken up and assigned to a delegate to run the methods sequentially.

Working on huge projects, what will the implications be of using a lot of delegates to run methods sequentially? Should I be concerned for using a lot of delegates in my code?

Example-

private delegate void Factory();
    Factory factory = null;
    protected void Page_Load(object sender, EventArgs e)
    {
        try
        {
            factory += SetCatalogClientProxy;
            factory += SetHelper;
            factory += GetAccessToken;
            //factory += GetCatalog;
            factory += GetToken;
            factory.Invoke();
        }
        catch (Exception ex)
        {
            AddToResult(ex.Message + NEWLINE + ex.StackTrace.Replace(" at ", " at " + NEWLINE).Replace(" in ", " in " + NEWLINE));
        }
    }

    #region Test Process Methods

    private void SetCatalogClientProxy()
    {
        catalog = new JAM.CatalogClientProxy();
        AddToResult("CatalogClientProxy Successfully initialized");
    }

    private void SetHelper()
    {
        helper = new JAM.HelperClass();
        AddToResult("HelperClass Successfully Intialized");
    }

    private void GetAccessToken()
    {
        accessToken = JAM.CatalogClientProxy._accessToken;
        if (string.IsNullOrEmpty(accessToken))
        {
            catalog.GetACSClaim();
            accessToken = JAM.CatalogClientProxy._accessToken;
        }
        AddToResult("Accesstoken Retrieved." + NEWLINE + accessToken);
    }

    private void GetCatalog()
    {
        catalog.CallCatalogService("fullcatalog", utcTime, country, "2", false);
        AddToResult(helper.Catalog(baseUrl, utcTime, page, country, accessToken));
    }

    private void GetToken()
    {
        var Fulfillment = new JAM.FulfillmentServiceProxy(Guid.NewGuid().ToString());
        ClassImport.FulfillmentInfo result = Fulfillment.GetToken("ABC-11111", "123456", "ZA", "123456", "123456", JAM.CatalogClientProxy._accessToken);

        if (result != null)
        {
            var output = "Products" + NEWLINE;
            foreach (var p in result.Products)
            {
                output += "    " + p.ProductName + "<br/>";
                foreach (var l in p.Links)
                {
                    output += "        " + l.Uri + "<br/>";
                }
            }
            AddToResult(output);
        }
        else
        {
            AddToResult("No products Found");
        }
    }
    #endregion

    #region Logging To Page

    private void AddToResult(string text)
    {
        result.InnerHtml += String.Concat("<strong>[", DateTime.Now.ToString(), "]</strong> ", text, NEWLINE);
    }

    #endregion

This is from one of my test pages. There is only 1 delegate here, but on other projects I work on I sometimes have 5 or 6 delegates set up similarly to perform methods sequentially, where I try to keep the methods at 1 action each.

Answer 1

I have been told that a good practice for writing methods is to make the method only responsible for doing one thing.

The common problem people have with this is that they take it too literally. If a function can only ever do one thing, how do you ever do more than one thing ever?

If your "one thing" takes 5 steps, then make a single function that does these 5 steps, and only those 5 steps. In turn, each of those steps should be a function that does that step, and only that step (which in turn may be 3 smaller steps).

A single responsibility necessarily requires multiple smaller responsibilities or else you wouldn't be writing a method... you'd just use the smaller responsibility directly.

Working on huge projects, what will the implications be of using a lot of delegates to run methods sequentially?

• Delegates tend to be a weaker contract than other associations. Since it can do anything as long as it meets the delegate signature, your code can become very fragile and hard to reason about.
• I'd say... half of C# developers or so don't even know that delegates are multicast.
• The exception handling behavior in multicast delegates isn't going to be what you always want.
• The optimizer (and JITer) can do far, far less with delegates compared to a real function. The impact will vary depending on your compiler and target runtime. Measure to be sure.

Answer 2

It's really a bad idea to overuse delegates. Reasons are mainly about readability. Delegate's name can be changed to any name when it is passed in between classes without affect it's function. For example:

FunctionA(){
    Action<int> print = (x) => Console.WriteLine(x);
    FunctionB(print);
}

FunctionB(Action<int> tellMeWhatTheIntValueIs){
    tellMeWhatTheIntValueIs(5);
}

If you want to know what is "tellMeWhatTheIntValueIs" do to the number "5", F12 from Visual Studio won't work. You need to trace the caller of functionB.

After few months, engineer1 and engineer2 come and want to reuse the delegate "print". They can do thing like the following:

FunctionC(Action<int> showMeTheNumber){
    showMeTheNumber(5);
}

FunctionD(Action<int> jibajiaba){
    jibajiaba(5);
}

One delegate has 4 names in this case!

You may won't jump into this situation if both of the functions are written by you, or written in the same time. But I saw so many cases when people pass delegate into another method, they want to reuse the "print" delete, then they modify its name for whatever reasons.

However, if you do it with an object. F12 on "Print" method will route you to the implementation part, then you can back to functionB easily. It's more readable.

Class Printer{
    void Print(int number){
        Console.WriteLine(x);
    }
}

FunctionA(){
    FunctionB(new Printer());
}

FunctionB(Printer printer){
    printer.Print(5);
}

FunctionC(Printer showMeTheNumber){
    showMeTheNumber.Print(5);
}

FunctionD(Printer jibajiaba){
    jibajiaba.Print(5);
}

What's worse?

The following codes will happen!

FunctionA(){
    Action<int> y = x => Console.WriteLine("This is " + x);
    Action<int, Action<int>, int> print = (x, y, z) => {
        x();
        y(z);
        Console.WriteLine(x);
    };

    FunctionB(print, y, 5);
}

FunctionB(Action<int, Action<int>, int> tellMeWhatTheIntValueIs, Action<int> printTheNumber, int theNumber){
    tellMeWhatTheIntValueIs(theNumber, printTheNumber, theNumber);
}