Can a C# method chain be "too long"?

Question

Not in terms of readability, naturally, since you can always arrange the separate methods into separate lines. Rather, is it dangerous, for any reason, to chain an excessively large number of methods together? I use method chaining primarily to save space on declaring individual one-use variables, and traditionally using return methods instead of methods that modify the caller. Except for string methods, those I kinda chain mercilessly. In any case, I worry sometimes about the impact of using exceptionally long method chains all in one line.

Let's say I need to update the value of one item based on someone's username. Unfortunately, the shortest method to retrieve the correct user looks something like the following.

SPWeb web = GetWorkflowWeb();
SPList list2 = web.Lists["Wars"];
SPListItem item2 = list2.GetItemById(3);
SPListItem item3 = item2.GetItemFromLookup("Armies", "Allied Army");
SPUser user2 = item2.GetSPUser("Commander");
SPUser user3 = user2.GetAssociate("Spouse");
string username2 = user3.Name;
item1["Contact"] = username2;

Everything with a 2 or 3 lasts for only one call, so I might condense it as the following (which also lets me get rid of a would-be-superfluous 1):

SPWeb web = GetWorkflowWeb();
item["Contact"] = web.Lists["Armies"]
                     .GetItemById(3)
                     .GetItemFromLookup("Armies", "Allied Army")
                     .GetSPUser("Commander")
                     .GetAssociate("Spouse")
                     .Name;

Admittedly, it looks a lot longer when it is all in one line and when you have int.Parse(ddlArmy.SelectedValue.CutBefore(";#", false)) instead of 3. Nevertheless, this is one of the average lengths of these chains, and I can easily foresee some of exceptionally longer counts. Excluding readability, is there anything I should be worried about for these 10+ method chains? Or is there no harm in using really really long method chains?

Answer 1

There are no technical limitations on how long a method chain can be.

However, three areas that can become problemattic are debugging, exception handling, and resource disposal.

Debugging is complicated by the same fact that makes chaining so elegant - the lack of intermediate temporary variables. Unfortunately, without temp variables, inspecting intermediate results when debugging becomes painful.

Exception handling is complicated by the fact that you cannot isolate exceptions raised from one method vs. another. Normally this isn't an issue if you can't do something meaningful in response to the exception - just let it propagate up the call chain. However, if you do realize at a later point that you need exception handling, you have to refactor the chaining syntax to be able to insert appropriate try/catch handlers.

Similar to exception handling is the case of deterministic disposal of resources. The easiest way in C# to accomplish this is with using() - unfortunately, chaining syntax precludes that. If you are calling methods that return disposable objects, it probably a good idea to avoid chaining syntax so that you can be a good "code citizen" and dispose of those resources as early as possible.

Method chaining syntax is often a used in fluent APIs, where it allows the syntax of your code to more closely reflect the sequence of operations you intend. LINQ is one example in .NET where fluent/chaining syntax is often seen.

Answer 2

The only consern you should consider regarding this (if yor ignoring readability) is resource handling and GC.. Questions you should ask are.

• Are the calls I am making returning objects that should be disposed of?
• am I initing alot of stuff inside a single scope rather than smaller scopes that GC might be more reactive to? (GC is scheduled to run each time you leave a scope, though scheduling and when it actually runs are two different things :-p).

But really.. weather you call one method per line, or string them all togeather its all ends up being the same in IL (or very nearly the same).

Josh

Answer 3

Readability is the biggest concern, but often that isn't a problem at all.

You could also describe LINQ query syntax as (underneath it all) exactly such a setup. It just makes it look prettier ;-p

One possible issue is where you need to introduce things like using or lock; with the fluent API you may be tempted to simply drop these components, but that might lead to oddities when an exception is thrown.

The other possible thought is that you might want to have more granular exception handling around some of the calls; but you can always just break the flow:

var foo = bar.MethodA().MethodB(...).MethodC();
try {
    foo.MethodD();
} catch (SomeSpecificException) {
    //something interesting
}

Or you could even do that in an extension method to keep the fluent appearance:

bar.MethodA().MethodB(...).MethodC().MyExtensionMethodD();

where MyExtensionMethodD is the one you add with special handling (exceptions, locks, using, etc).

Answer 4

This is considered a code smell by some and not by others. Anytime you see the following:

Foo.getBar().getBlah().getItem().getName();

you should really be thinking, "what do I really want?" Instead perhaps your method should contain a function call:

String getName(Int _id, String _item)
{
    return myBar.getName( _id, _item );
}

which then delegates downward, amongst the classes. Then, if something changes in one of your classes at a later update, you'll see exactly where it occurs and can change it in one location.