• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Looking for a way to improve this code

 
Ranch Hand
Posts: 67
1
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.


   
 
Bartender
Posts: 732
10
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Do you know how to use
 
lowercase baba
Posts: 13089
67
Chrome Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Rancher
Posts: 5008
38
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 5008
38
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 5008
38
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 732
10
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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:
 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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!
 
Java Cowboy
Posts: 16084
88
Android Scala IntelliJ IDE Spring Java
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Posts: 13089
67
Chrome Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Marshal
Posts: 79151
377
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Bartender
Posts: 2236
63
IntelliJ IDE Firefox Browser Spring Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
This conversation is getting deep
 
Bartender
Posts: 1205
22
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 2236
63
IntelliJ IDE Firefox Browser Spring Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
There is no ||= operator in Java...
 
Zahro Alfelpha
Ranch Hand
Posts: 67
1
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
 
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Bartender
Posts: 1205
22
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Spring Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Bartender
Posts: 1205
22
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Bartender
Posts: 1205
22
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 79151
377
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Don't get me started about those stupid light bulbs.
reply
    Bookmark Topic Watch Topic
  • New Topic