As a rule, How many function chains per line is too complicated?
By Function chaining I mean:
Obj.Display().Show().Result()
What does your team use a guideline?
As a rule, How many function chains per line is too complicated?
By Function chaining I mean:
Obj.Display().Show().Result()
What does your team use a guideline?
As a rule, whatever you and your other dev team members can tolerate.
I think two is too many. Code is a lot more readable and robust if you actually show what those functions are returning, maybe do some validation to check for null, etc, and then call the next function.
Code will be read many, many times more often than it will be written. Verbosity can be a good thing.
It depends to some degree on the language, too. While it may be "bad form" to do a long chain in, say, Java, it's considered good fun in Ruby.
It depends, but usually when reusing parts of the chain it can be easier to read if cached:
var s = Obj.Display().Show();
s.Result();
s.SomethingElse();
Instead of:
Obj.Display().Show().Result();
Obj.Display().Show().SomethingElse();
I think more than two is stretching it. The reason is, if any of the methods happen to return null
and a NullPointerException
(or equivalent) is thrown, the stack trace does not point you straight to the bug.
I suppose I could make exceptions for cases when the methods are documented to never return null
, honor that contract, and can be proven to honor that contract. In general, fewer is better, IMHO.
Unless it is a fluent interface, two is too many. It is because if you need to do such a thing, this probably means you need some service from an object you have no explicit dependency on.
Fluent interfaces are different because you are chaining method calls to the same object or a group of objects designed for such an usage.
EDIT: correction about fluent interfaces
I may be the only one here, but personally I really hate method chaining. For each dereferencing of a result value I see possible null pointers and exceptions thrown mid-stream. And often enough there is no catch block anywhere to help with either of those.
In C/C++ your code will not even throw an exception but crash on a NULL pointer.
Chaining is okay as long as you as know what you are doing and what can happen in a bad case. I personally never like to wrap everything in a try/catch-block.
I have a personal preference that is limited by number of characters, not number of calls.
I try to cap my line length at 80-110 characters which, by definition, limits the number of chained method calls I can make.
That being said, chained method calls unwittingly add complexity and make it harder to debug your code.
Read up on the Law of Demeter. Chaining/complexity leads to trouble.
The Law of Demeter suggests you should use only one dot for maintainability. The upside is that you enforce better encapsulation if the inner methods aren't exposed. The downside is that you end up writing more wrapper methods.
I personally like to follow 2 rules on chaining:
Shouldn't you be using getters and setters here anyway, so:
Obj.Display.Show.Result
Otherwise, the problem I have with this code is that your function names are verbs, but they don't describe the return type, so it is harder to read. Would it be better if the function names had a verb and a noun, something like:
Obj.createDisplayBox().createShowWindow().addResult();
I think this many dots would be acceptable if you didn't intend to reuse any of the returned objects in the chain
One good programming practice is to step through everything you write in the debugger. As you do this, function chaining makes it hard to see what intermediate values get produced. Of course you can step into each call, but that's slower, and even then you might not find a variable to examine. (Some debuggers let you look at return values using a special symbol, but not all of them do.)
Similarly, if you're debugging a problem, it's much harder to select between step-into and step-over if you're in the middle of a big function chain. I always hit the wrong one and then have to start over, or else go set an extra breakpoint that will annoy me later.
So if you use an interactive debugger then it can be easier to be verbose and write some intermediate temporary variables than to do any chaining at all.
Using resharper I can pretty much mouse-over any part to know more about it, so I'm comfortable writing a few chains and you won't hear me whining about not knowing the types involved, but just not repetitive ones like:
Obj.Display().Show().Result();
Obj.Display().Show().ResultExpanded();
Now, with some of our objects the code ends up looking more like:
RegInfo.SiteID = int.Parse(ConfigManager.getInstance().getConfig().getSingleProperty(Constants.SITEID_KEY));
[not my code] Sometimes when it's really long it gets broken into bits:
RegInfo.SiteID = int.Parse(
ConfigManager.getInstance().getConfig()
.getSingleProperty(Constants.SITEID_KEY)
);
As others have pointed out, it depends on the nature of your methods. If there's a chance that any of the method calls can fail, don't chain them. However, if the API is specifically designed for safe method chaining, chain them as much as you like.
The Hibernate Criteria API is a good example of where extreme chaining is safe and actually makes the code more readable -- provided that it's nicely indented. Here's a sample from http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Criteria.html:
List cats = session.createCriteria(Cat.class)
.setProjection( Projections.projectionList()
.add( Projections.rowCount() )
.add( Projections.avg("weight") )
.add( Projections.max("weight") )
.add( Projections.min("weight") )
.add( Projections.groupProperty("color") )
)
.addOrder( Order.asc("color") )
.list();
Also note that in C++, STL streams rely on method chaining to allow the following syntax:
cout << "All your base are belong to " << strWhom << endl;
Needless to say, it's absolutely OK to use extensive chaining here; in fact, it makes the reading of the code more natural. I don't think there's a general rule for the maximum number of chains beyond which code would be rendered unreadable. It all depends on the context.
The readability rule is definitely a good start. On the last team I worked on several of the developers were greatly confused by such method chaining and as a result I was forced to rewrite my code so that they could read it. Try out different examples then have various team members read it, .NET LINQ example might be a good start or any other fluent interface. If they are having trouble reading it or after several attempts to share it with them it is still being rejected then you may want to keep your chaining to a minimum. Try to strike a balance and good luck.