3

In .Net, you can chain methods returning a value or by using a void. Is one of them the "right way"?

So you could say

1) 

Foo myFoo = new Foo();
myfoo.Bars = 
  myBars.DoSomethingCool(x)
  .DoSomethingElse(y)
  .AndSomethingElse(z);

public static IList<IBar> DoSomethingCool(this IList<IBar> source, object x)
{
  IList<IBar> result = //some fn(source)
  return result;
}

In this case, all 3 extension methods need to return IList (the type for myFoo.Bars)

or it could also be written as

2)

myBars.DoSomethingCool(x)
.DoSomethingElse(y)
.AndSomethingElse(z);

public static void DoSomethingCool(this IList<IBar> source, object x)
{
  //Modify source
  source = //some fn(source)
  //Don't return anything
}

in which case, the extension methods return a void, but do the work on the source object that's coming in?

UPDATE Simon was correct in his answer that 2) won't compile. Here is how that could be re-written:

DoSomethingCool(myBars)
.DoSomethingElse(myBars)
.AndSomethingElse(myBars);

myBars would then be changing inside the call of each method and the methods would be returning void.

John
  • 3,332
  • 5
  • 33
  • 55
  • 3
    Chaining methods makes code harder to debug, modify and maintain. – Steve Wellens Jul 14 '11 at 02:33
  • I'm not sure that I understand how it makes debugging, modification and maintenance harder. If you put the chaining on separate lines, you can set a break point at any of the statements. Chaining makes it easier to express intent, which would seem to me to make maintenance easier, not harder. – John Jul 14 '11 at 13:58
  • Your second example wont compile. DoSomethingElse() is called on the return result of DoSomethingCool, which is a void. – sisve Jul 14 '11 at 14:56
  • 2
    Chaining is bad because you can't single step through each logical line of code...it's all or nothing. You can't put a breakpoint on a step. If a step fails you don't know which step it was. Having all the code on one line makes it more difficult for the eyes to parse the separate steps. To illustrate, put every single step in your program on ONE LINE. Do you like it? Putting each logical step on a single line is much better. – Steve Wellens Jul 14 '11 at 16:01
  • Simon, you're right. I have to return a type, otherwise I can only chain the result once. – John Jul 14 '11 at 16:35
  • Steve, I edited the post a couple hours ago to put the chaining statements on separate lines. Perhaps you are looking at the previous code? – John Jul 14 '11 at 16:40
  • Steve, my bad, I see what you're saying. I thought that I remembered setting breakpoints on the individual lines, but I was mistaken. You have to set the breakpoints within the Extension methods themselves, which is a bit of a nuisance, but can still be done. The question then becomes which is more important, the extra readability of the fluent style of chaining vs the additional time that it takes to debug. – John Jul 14 '11 at 16:49

2 Answers2

0

In both cases you're using an extension method, returning an object, and then using that object as the input to the next method in the chain. The second example is close to a Fluent Interface (but not quite). I think in your two examples the critical distinction is one of mutability. Do you want the original input to be modified (the second example) or not (the first example)? The "correct" answer depends on the situation and context.

daveaglick
  • 3,600
  • 31
  • 45
  • That's the question: is it ok to modify the original input or is doing manipulation through side effects a bad thing? – John Jul 14 '11 at 13:54
  • I don't know that modifying through extension methods this way is necessarily a "bad thing", though I might personally stay away from it to avoid confusion - at a minimum I would name methods that mutate the object with something like "Change..." or "Modify...". By convention nearly all of the Linq extension methods operate on iterators and provide a new sequence rather than modifying the original one. By operating on and returning the lowest common denominator (iterators) the Linq extensions can adapt to the widest variety of situations and target objects. – daveaglick Jul 14 '11 at 15:56
  • One thing to keep in mind though is that extension methods are really no different than any other method, the first parameter is just implicit instead of explicit. The implication is that because it's external to the object being operated on, the method can't have any more access than any other one. If the original object is not mutable, then the method(s) in the chain cannot mutate it either. This means you're not in danger of breaking encapsulation through either approach as long as the original object's interface was designed appropriately. – daveaglick Jul 14 '11 at 15:58
0

1) wins. Simon correctly answered that 2) wouldn't compile, but he did it as a comment, so I can't gie him credit for answering. The updated solution for 2) does everything as a side effect an I can't think of a reason why you would ever want to do that in a statically typed language.

The points made about chaining's debugging issue are something to consider, though I find chaining to be especially useful when filtering.

mybars.FilterByHasHappyHour() is much nicer than

BigGiantUtilityClass.GetBarsWithHappyHours(myBars)
John
  • 3,332
  • 5
  • 33
  • 55