1

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.

  • 3
    Can you give an example of a method that does multiple things and how you would refactor it with delegates for running methods sequentially? – Bart van Ingen Schenau Jul 16 '14 at 13:15
  • So that's already one thing. Not sure How I will refactor it. That's one setback. I need to know more setbacks :) – Eon Jul 16 '14 at 13:23
  • 6
    Your Page_Load baffles me. Why wouldn't you simply call each method in sequence? (And less importantly, why would you call something that doesn't return a value a factory? That's just plain misleading, and there's already a delegate for that.) – Doval Jul 16 '14 at 13:38
  • 2
    Why bother with delegates, when you can just call the method itself?? – Euphoric Jul 16 '14 at 13:42
  • It's not the page load that matters here, my question is about delegates - are there drawbacks to over-using them? – Eon Jul 16 '14 at 13:42
10

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.
|improve this answer|||||
  • 5
    If you're using multicast delegates outside of events, you're most likely doing something wrong. – svick Jul 16 '14 at 14:53
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);
}
|improve this answer|||||
  • While I agree with the message of the first half of this answer, life isn't so bad, because C# has nominal delegates (i.e. not everything needs to be a Func<int> to return an int; you can define your own int returning delegate), and you probably meant Action<int> rather than Func<int>.var y = (implicitly-typed lambda/delegate) doesn't compile. – VisualMelon Mar 1 '19 at 12:34
  • @VisualMelon haha, my fault. "var" is innocent. – Frank Gao Mar 2 '19 at 23:43
  • @VisualMelon corrected. – Frank Gao Mar 2 '19 at 23:45
  • This doesn't compile: Action<int, Action<int>, int> y = x => Console.WriteLine("This is " + x);; to compile the implicitly typed lambda must structurally match the delegate type, which this does not. The call to FunctionB won't work either (C# doesn't support tuple->argument splatting). Your code is also missing a ; and method return arguments, and the whole print assignment is implicit (i.e. varesq), so won't work at all. – VisualMelon Mar 3 '19 at 1:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for?Browse other questions tagged or ask your own question.