Create new variable or make multiple chained calls?

Question

What is the best way to get this attributes, thinking in performance and code quality?

Using chained calls:

 name = this.product.getStock().getItems().get(index).getName();
 id = this.product.getStock().getItems().get(index).getId();

Creating new variable:

 final item = this.product.getStock().getItems().get(index);
 name = item.getName();
 it = item.getId();

I prefer the second way, to let the code cleaner. But I would like to see some opinions about it.

Thank you!

• deadalnix: OOP say tell, don't ask. Both proposals are anti patterns. Some company even prevent getter to avoid falling in that trap.
• James: Both are pretty much disgusting but the second is less disgusting.
• Brian: I feel like you should be able to replace this.product.getStock().getItems().get(index).getName(); with product.getStock()[index].getName(); Next step would be something like foreach (product in product.getStock()){name = product.getName();id = product.getId();/*Do Stuff*/}. Of course, this is language dependent.

Answer 1

Your chained approach isn't so bad (I admit I've done it for short and simple chains) but the question is what happens when one of the items in the middle of this get-chain you're trying to get returns null?

I recently had to debug something similar and it was a pain. I eventually split it up into something like this:

a = incomingObject.getA();
b = a.getB();
c = b.getC();

...well actually the chain I was dealing with was 7 gets deep. And then once that was resolved (tried and caught) I discovered that a different method (not a get, but it was used like one) was throwing a webservice exception. Re-writing it as multiple statements was tedious but much easier to debug when things went wrong.

Answer 2

I like niether, personnaly.

If any of them returns a null you get a nullpointer exception on the line...

Now... which one of them did go wrong ?

Now, method chaining can be and is very usefull with some specific usage patterns. Say for example in a fluent type interface (most mocking framework will do this, at least the ones I`ve seen).

Ideally one should strive to reduce coupling and this pattern does exactly the opposite.

From experience when I find myself having to dig deep in my object structure to do a task it usually is an indicator that my object structure or my business logic is not right. Perhaps the work could be buried deeper through some kind of wrapper object / facade / delegate and just call that instead. Perhaps your function or method is doing too much and should be split in smaller, more specialized chunks. Perhaps the object needs to expose properties of the items inside instead of exposing the item as a whole. It all depends on your architecture and use cases.

That said, software needs to be shipped and that usually means some parts will be more hackish than others, Ideally these should be well isolated bounded and boxed and method chaining like this weather done through successive variable assignement or directly tends to break these boundaries reducing the overall stability.

Answer 3

I prefer the second way as well because it shows intent. You are getting the name and it off of a single item you have retrieved.

In the first way, could there exist a possibility (e.g. via another thread) that .get(index) returns a different item between the two calls? If that could happen now, or in some future iteration of the software, the second way not only has the benefit I talk about above, but it may become necessary.

Answer 4

Well I think that the second option you posted is cleaner, and sometimes the compiler might just do it for you in the background as an optimization. Because one pointer jump (or function call) is better that 5. So the code is cleaner and quicker.
So the only question you should ask yourself is: Could it be that one object in the chain changes its result? If the answer is No, just go with the short and nice way. If its Yes, its simply the correctness of the program.

Edit

I was just studying for an exam, and came across the Introduce Explaining Variable refactoring, which reminded me of your question. I think you will find the discussion there interesting. The main points are:

• Your second option is the preferred one.
• If Extract Method is applicable, do it instead. And create a general accessor method for the desired objects. (That way you have more readable code, and less copy-paste code)