• Post Reply Bookmark Topic Watch Topic
  • New Topic

Looking for a way to improve this code  RSS feed

 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In the following code I only want to call a method after the loop if one of the properties in the list contain a certain value.
It seems silly to just set the boolean over and over again when just setting it once would work, but I can't think of any other way to do it.
I am using Java 1.5.


   
 
Fred Kleinschmidt
Bartender
Posts: 571
9
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Do you know how to use
 
fred rosenberger
lowercase baba
Bartender
Posts: 12563
49
Chrome Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
break may or may not work, depending on what someMethod1 or someMethod2 do.
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Fred Kleinschmidt wrote:Do you know how to use


Indeed I do know how to use break, but someMethod1() and someMethod2() still need to run for every loop.
 
Norm Radder
Rancher
Posts: 2240
28
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
someMethod1() and someMethod2() still need to run for every loop.

That implies that comments would be useful for anyone reading the code to understand what the code is supposed to do.
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Norm Radder wrote:
someMethod1() and someMethod2() still need to run for every loop.

That implies that comments would be useful for anyone reading the code to understand what the code is supposed to do.


Not quite sure what you're trying to say
 
Norm Radder
Rancher
Posts: 2240
28
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
what you're trying to say

Fred assumed that the purpose of the loop was to find a dino that was purple.  At that point he thought the loop could be exited with a break.
Comments describing what the loop was supposed to do would prevent that confusion.
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Norm Radder wrote:
what you're trying to say

Fred assumed that the purpose of the loop was to find a dino that was purple.  At that point he thought the loop could be exited with a break.
Comments describing what the loop was supposed to do would prevent that confusion.


Oh, gotcha. So something like this then?

 
Norm Radder
Rancher
Posts: 2240
28
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Or something like this:
Both someMethod1 and someMethod2 need to be called with all the dinos from dinosaurs.
Also all the dinos need to be tested to see if any one is purple.
 
Fred Kleinschmidt
Bartender
Posts: 571
9
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You could always wrap the check in an if-statement, but that doesn't really save much unless getColor.equals("purple") turned out to be expensive to execute:
 
Junilu Lacar
Sheriff
Posts: 11485
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I would try to refactor to this if it makes sense:


I have no idea what someMethod1() and someMethod2() actually do but I wouldn't be surprised if they are actually more appropriate to be methods of a Dinosaur object. Introducing the method doStuff() that calls someMethod1 and someMethod2 is another way of abstracting away details and simplifying higher-level code.

You might now question the choice to iterate over dinosaurs twice instead of just once. That may or may not be an issue but clarity of intent and separation of concerns should be a priority. Trying to prematurely optimize your code usually doesn't give you any real benefits but it often does result in messier and more entangled code.
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for the replies. I don't really like the idea of looping through the list twice because there could be many many dinosaurs. I will look into that though and see if I can come up with something. I guess this would depend on what I think the ratio of purple dinosaurs would be. If I think there will always be a large amount of them, then looping through a second time and quickly hitting a break might not be a bad idea. For my example, the someMethod calls would not be methods of the dinosaurs themselves, so that suggestion doesn't help me, but thank you!
 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16059
88
Android IntelliJ IDE Java Scala Spring
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In my opinion there's not a lot wrong with your original code, and I don't see an urgent need to improve it, or see any way that would significantly improve it. Setting the boolean each time you encounter a purple dino isn't something that costs any significant performance. If you don't like potentially setting the boolean multiple times, you chould include a check in the if-statement to see if the boolean has already been set, but that makes the code longer and doesn't really gain you anything:

 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jesper de Jong wrote:you could include a check in the if-statement to see if the boolean has already been set, but that makes the code longer and doesn't really gain you anything


This is actually what I ended up doing for now. It just irks me to set the same variable over and over again, and part of me thinks that the boolean comparison has got to be slightly faster than the string comparison, even on some negligible level. 
 
Junilu Lacar
Sheriff
Posts: 11485
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Zahro Alfelpha wrote:
This is actually what I ended up doing for now. It just irks me to set the same variable over and over again, and part of me thinks that the boolean comparison has got to be slightly faster than the string comparison, even on some negligible level. 

You are optimizing by gut feelings. Programmers are notoriously bad at optimizing using that method. I doubt you're an exception.

"Premature optimization is the root of all evil."

That quote is often attributed to Donald Knuth but I recently read somewhere that he got it from C.A.R. Hoare.
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:"Premature optimization is the root of all evil."


I like this. Thank you.
 
Junilu Lacar
Sheriff
Posts: 11485
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you really want to stick with one loop, you might try using a profiler to see if this gives you any significant gain in performance:


My gut tells me this might be more efficient. I could be wrong. (See my previous point)
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:



I'm not familiar with this syntax of using || in an assignment like that. Care to elaborate a little?
 
Junilu Lacar
Sheriff
Posts: 11485
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Since containsPurpleDino is a boolean, it can be assigned the value of a boolean expression.

The boolean expression containsPurpleDino || dino.isColored(PURPLE) will be short-circuited if containsPurpleDino is already true. That is, the part after the || operator will not be evaluated if containsPurpleDino is already true, so you save all those method calls to check if dino is PURPLE once you find one that is and set containsPurpleDino to true. The only thing that you don't avoid with this technique is that pesky reassignment.  But again, I think that's a very negligible cost.

If you tried to use a profiler to see how much overhead you're really looking at by iterating over the collection twice, you might also find that's not too bad either.  If there are many elements and if there are many purple dinos, the more purple dinos there are, the more likely you will not actually iterate over the whole collection. Worst case scenario, you iterate over the entire collection and find no purple dinos. Best case, the first dino you examine is purple.
 
fred rosenberger
lowercase baba
Bartender
Posts: 12563
49
Chrome Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
well...the || operator has precedence.  so you evaluate "containsPurpleDino || dino.isColored(PURPLE)", which results in a boolean value.

That value then gets assigned to your containsPurpleDino variable.
 
Junilu Lacar
Sheriff
Posts: 11485
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Zahro Alfelpha wrote:
Junilu Lacar wrote:


I'm not familiar with this syntax of using || in an assignment like that. Care to elaborate a little?

I just noticed the irony of your question in the light of your signature: toBe() || !toBe()

 
Junilu Lacar
Sheriff
Posts: 11485
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
There's also some irony in the fact that the expression toBe() || !toBe() will always be true
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:There's also some irony in the fact that the expression toBe() || !toBe() will always be true


Haha! I didn't even think of that.
 
Campbell Ritchie
Marshal
Posts: 56536
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The original quote from The Bard would come out as exclusive or because the two options are mutually exclusive:-
toBe() ^ !toBe()
It is still a tautology.
 
Paweł Baczyński
Bartender
Posts: 2083
44
Firefox Browser IntelliJ IDE Java Linux Spring
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:There's also some irony in the fact that the expression toBe() || !toBe() will always be true

This unjustly assumes toBe() always returns the same value.
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This conversation is getting deep
 
Ryan McGuire
Ranch Hand
Posts: 1143
9
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:If you really want to stick with one loop, you might try using a profiler to see if this gives you any significant gain in performance:


My gut tells me this might be more efficient. I could be wrong. (See my previous point)


You could convert this to...


Some would call that a "trick".  I prefer "technique" - it's perfectly valid syntax and is exactly what the ||= operator was implemented for, so it shouldn't be considered esoteric at all.
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ryan McGuire wrote:
Junilu Lacar wrote:If you really want to stick with one loop, you might try using a profiler to see if this gives you any significant gain in performance:


My gut tells me this might be more efficient. I could be wrong. (See my previous point)


You could convert this to...


Some would call that a "trick".  I prefer "technique" - it's perfectly valid syntax and is exactly what the ||= operator was implemented for, so it shouldn't be considered esoteric at all.


Oh wow, I had no idea this operator even existed!
 
Paweł Baczyński
Bartender
Posts: 2083
44
Firefox Browser IntelliJ IDE Java Linux Spring
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
There is no ||= operator in Java...
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paweł Baczyński wrote:There is no ||= operator in Java...


Well I suppose that's why I've never heard of it then
 
Liutauras Vilda
Sheriff
Posts: 4917
334
BSD
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Zahro Alfelpha wrote:It seems silly to just set the boolean over and over again when just setting it once would work
And you don't really need to.

The goal is to be aware about the purple dino if there is such, so, I'd naturally approach this way first:
And I would try to do something with methods someMethod1 and someMethod2 if I were to know what they mean.

Line 6 better would be to make more object oriented, but I left this way as it fits to the whole code style.
 
Ryan McGuire
Ranch Hand
Posts: 1143
9
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paweł Baczyński wrote:There is no ||= operator in Java...


Well that's embarrassing.  My only defense is that C# sharp has had that operator for a number of versions and I figured surely Java would've caught up on that by now.
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Java Linux Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Liutauras Vilda wrote:I would try to do something with methods someMethod1 and someMethod2 if I were to know what they mean.


Unfortunately, my actual code has nothing to do with dinosaurs or the color purple. I just threw those methods in there to signify that something was required to happen in every iteration.
 
Ryan McGuire
Ranch Hand
Posts: 1143
9
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ryan McGuire wrote:
Well that's embarrassing.  My only defense is that C# sharp has had that operator for a number of versions and I figured surely Java would've caught up on that by now.


Wait wait wait... Apply my same argument to the |= operator.  That operator works as I would expect when working on booleans.

 
Liutauras Vilda
Sheriff
Posts: 4917
334
BSD
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ryan McGuire wrote:That operator works as I would expect when working on booleans.
It doesn't.

Check this snippet:
What value 'b' you'd expect?
 
Ryan McGuire
Ranch Hand
Posts: 1143
9
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Liutauras Vilda wrote:
Ryan McGuire wrote:That operator works as I would expect when working on booleans.
It doesn't.

Check this snippet:
What value 'b' you'd expect?


Excellent point, and that's a bit disappointing.  In this case I merely wanted more concise source code and wasn't really worried about short-circuiting the boolean OR operator, and I would say that on that count, my shortened code "works". However, I would call not having a boolean OR (and AND) assignment operators that short circuits reasonably an "opportunity missed".
 
Campbell Ritchie
Marshal
Posts: 56536
172
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You can read about the compound assignment operators in the Java® Language Specification (=JLS). Beware: the JLS is notorious for being difficult to read. It would appear that the value of the right operand is calculated for all compound assignment operators and then the operator executed, then the assignment. Obviously they thought there was no need for ||= as well as |=. Having both operators might violate the “simple” buzzword.
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!