• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

which of the two is good practice?

 
Pavan Kumar
Ranch Hand
Posts: 78
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This may be a very basic thing, but I want to hear your opinion.

Thanks for your time.

option 1

Option 2


I looked at the byte code, and looks like monitor is released in both the cases. Which one do you prefer to write and why?

Thanks,
Cnu
 
Ernest Friedman-Hill
author and iconoclast
Marshal
Pie
Posts: 24212
35
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Given that ArrayList itself isn't synchronized, I'm assuming that "listeners" is a synchronized collection like a Vector or Collections.synchronizedList(), or this is really meaningless.

I wouldn't be surprised to find that the bytecode for both is identical. The only difference, then, is clarity. I prefer the first, since it's shorter but no more complex.
 
Layne Lund
Ranch Hand
Posts: 3061
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Ernest Friedman-Hill:
Given that ArrayList itself isn't synchronized, I'm assuming that "listeners" is a synchronized collection like a Vector or Collections.synchronizedList(), or this is really meaningless.

I wouldn't be surprised to find that the bytecode for both is identical. The only difference, then, is clarity. I prefer the first, since it's shorter but no more complex.


Umm...the second looks shorter to me...or maybe I can't count...
 
Guy Allard
Ranch Hand
Posts: 776
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by cnu sri:
This may be a very basic thing, but I want to hear your opinion.
..........


So what are the differences in the byte code?

G.
 
Ernest Friedman-Hill
author and iconoclast
Marshal
Pie
Posts: 24212
35
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Layne Lund:

Umm...the second looks shorter to me...or maybe I can't count...


Sorry. I should have just said "the shorter one."
 
Ernest Friedman-Hill
author and iconoclast
Marshal
Pie
Posts: 24212
35
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I used javap to check it out. Both jikes and javac do generate slightly different code for the two routines. The shorter one does give rise to slightly shorter bytecode. Basically, for the short one, the return value is loaded onto the stack before the lock is released; for the longer one, it's loaded afterwards. The difference is very slight.
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I prefer Option 2, simply because it's easier to read and understand. Option one takes me a couple of seconds more to parse, and why waste any brain cells on that?
 
Marilyn de Queiroz
Sheriff
Posts: 9066
12
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I prefer option 1 for the same reason that Ilja prefers option 2.

Also, I think that returning in the middle of a method rather than at the end, particularly if there is more than one return in the method, makes it more difficult to refactor later.
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Marilyn de Queiroz:
I prefer option 1 for the same reason that Ilja prefers option 2.


That's interesting - could you please elaborate on what you find easier to parse about option 1?

Here is what happens to me:

Option 1: "OK, there is some synchronized block. After that, iter gets returned. What is iter? Ah, iter gets assigned to in the synchronized block. What's that line above. Duh, it's just the declaration of iter. Let's double check that it's really the same variable. Yes, seems to be. So let's summarize: it returns an iterator for the listeners, wrapped by a new ArrayList."

Option 2: "OK, just a synchronized block with a return in it. Seems to return an iterator of listeners, wrapped by a new ArrayList."

The difference really is just a couple of seconds. Still, I find option 1 to be unnecessarily complex. It's also more likely that it contains a bug - just let's also have a field called itera, and return that one instead...

Also, I think that returning in the middle of a method rather than at the end, particularly if there is more than one return in the method, makes it more difficult to refactor later.


That's true, though I don't see how it's true in this case.

On the other hand I find that early returns can make methods easier to *understand*, which is a prerequisite for refactoring.

Often when I have to refactor a big method, one of the very first things I do is introducing Guard Clauses. After that, it's much easier to understand which steps the method can be separated into. Refactoring the method so that, for example, it's easy to extract methods, isn't too hard, even with the guard clauses present.

Your mileage may vary, of course.
 
Pavan Kumar
Ranch Hand
Posts: 78
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks very much for your replies.
I'm assuming that "listeners" is a synchronized collection like a Vector or Collections.synchronizedList()

Yes, indeed it's Collections.synchronizedList().

So what are the differences in the byte code?

I should probably have mentioned that I did not understand most of it,besides asking my question more clearly. but my concern was to find out if the monitor was released in both the cases. I was still not very assured and wanted to hear some more about the issue.

Thanks for your valueable comments and time.

it may not matter much, but here's the complete code,


Cnu,
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic