6

I'm working on a JSONObject with multiple sub-JSONObjects. Here is the way I fill the content :

myJson.getJSONObject(CAT_NAME).put(VAR_NAME, var)
                          .put(VAR_NAME2, var2)
                          .put(...);

A friend told me that it is a very bad practice to use "nested function/method calls" and that I should do it this way :

myJson.getJSONObject(CAT_NAME).put(VAR_NAME, var);
myJson.getJSONObject(CAT_NAME).put(VAR_NAME2, var2);
myJson.getJSONObject(CAT_NAME).put(...);

From my view, my way is more like an chained method call than a nested one. And I don't like the second way because it force the program to get the same object again and again when the put() method already returns it.

Is my case is a "nested function calls" case ? Is this dangerous or bad for any reason ? And what are those reasons ?

edit : I don't feel like my question is duplicated. The other question involves chained methods but it mainly talks about c# interfaces.

Rox Teddy
  • 101
  • 15
  • possible duplicate of [What's the point of DSLs / fluent interfaces](http://stackoverflow.com/questions/587995/whats-the-point-of-dsls-fluent-interfaces) – dhke Aug 13 '15 at 13:26
  • I can't tell if duplicated or not yet. Also I realize the answer will be very subjective. So I'll wait for a bit before taking any action. Thank you for your answers – Rox Teddy Aug 13 '15 at 13:33

3 Answers3

4

Is my case is a "nested function calls" case ?

No that is method chaining (Builder pattern).

Is this dangerous or bad for any reason ?

No. Your friend is wrong. Not at all bad practice in your case. It's quite Ok since you are building a Json.

Suresh Atta
  • 120,458
  • 37
  • 198
  • 307
2

Using method chaining will actually be more efficient than the alternative you provided because myJson.getJSONObject(..) is only called once in the first case, whereas you call it many times in the second. There is a much more significant cost for calling getJSONObject(..) than there is for reusing the original object returned by it.

The correct way to accomplish this without using method chaining would be more like this:

JSONObject obj = myJson.getJSONObject(CAT_NAME);
obj.put(VAR_NAME, var);
obj.put(VAR_NAME2, var2);
obj.put(...);

Personally, I prefer to not use method chaining because I think it looks better, but ultimately it's your preference and the code here would have basically the same performance as your first chunk of code.

Daniel Centore
  • 3,220
  • 1
  • 18
  • 39
  • You're doing the same as his first option. – arodriguezdonaire Aug 13 '15 at 13:53
  • @AndreuRodrígueziDonaire Yes, this is an equally efficient method as the first one while avoiding method chaining. Both my method and his first one are equally efficient and one is not better than the other. I was just showing why the second method was wrong and how to correct that. – Daniel Centore Aug 13 '15 at 13:56
  • I agree this is a good alternative. In my case there is no risk of getting wrong category because my json come from a template and i use static final for category names. But in a more generic situation, It could be good to test get() result before using put() on it. – Rox Teddy Aug 13 '15 at 14:02
  • @RoxTeddy The reason your second method is bad is because running getJSONObject(..) has a performance impact - it takes time to find that object. So, once you've found the object, it's faster to keep reusing it rather than looking for it repeatedly. – Daniel Centore Aug 13 '15 at 14:08
  • I'm not talking about exception handling. I'm telling you that the get method is SLOW. So you should only call it once and then reuse the result rather than doing a slow thing repeatedly. – Daniel Centore Aug 13 '15 at 14:11
  • Your first method and my only method both accomplish that. – Daniel Centore Aug 13 '15 at 14:12
  • @DanielCentore sorry I deleted my comment about exception catching because it is indeed a different problem. – Rox Teddy Aug 13 '15 at 14:14
  • 1
    @DanielCentore I agree. If we don't like chaining, your way is better than the 2nd I gave. – Rox Teddy Aug 13 '15 at 14:14
1

He PROBABLY considers it bad because it can be less readable (but that's a personal opinion and depends a lot on code formating, how well someone understands the specific APIs, is familiar with the concept of method chaining, etc. etc.).

It's certainly not a generally bad thing to do though. In fact a lot of APIs work exactly like that. Just look at StringBuilder in the Java standard API as a very commonly used example.
As others already pointed out it's potentially more performant (depending on how the called methods are implemented) as well, but that's not a given.

jwenting
  • 5,505
  • 2
  • 25
  • 30