wood burning stoves 2.0*
The moose likes Java in General and the fly likes Java Best Practice - Elegant code - Opinions please? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Java in General
Bookmark "Java Best Practice - Elegant code - Opinions please?" Watch "Java Best Practice - Elegant code - Opinions please?" New topic
Author

Java Best Practice - Elegant code - Opinions please?

conor o callaghan
Greenhorn

Joined: Oct 01, 2011
Posts: 5
This is just a question of taste. I don't expect everyone to agree with me, but is it just me or does anyone else here find this bit of code a little annoying.

I mean its perfect - it does what its meant to do but does anyone else not think it should be written slightly differently? If so how and why?

John Jai
Bartender

Joined: May 31, 2011
Posts: 1776
Does object changes for each iteration?
Mohamed Sanaulla
Saloon Keeper

Joined: Sep 08, 2007
Posts: 3068
    
  33

1. What is it that you want the code to achieve?
2. If its just a condition and no fixed number of iterations- you can choose while loop.
But without the context in which this code is used, why this piece of code is written, I dont think much can be told about the code.

Mohamed Sanaulla | My Blog
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7700
    
  20

conor o callaghan wrote:This is just a question of taste. I don't expect everyone to agree with me, but is it just me or does anyone else here find this bit of code a little annoying.

I mean its perfect...

Isn't that a tortological statement?

However, to answer your question, and with nothing else to go by:
1. Your code actually looks more like a while loop than a for.
2. Assuming (1), and that there are no other wrinkles you haven't told us about, you could rewrite it as:Why? Because it's (a) clearer (I happen to hate negative logic), and (b) faster.

HIH

Winston


Isn't it funny how there's always time and money enough to do it WRONG?
Articles by Winston can be found here
Peter Kirk
Greenhorn

Joined: Oct 12, 2011
Posts: 12
I'd probably just write it
for (cond) {
if (object.isProcessable()) {
other lines;
}
}

or a while loop as others have said, depending on what the iteration is. But I don't think
while (cond && object.isProcessable())

is neccessarily the same.

Rob Spoor
Sheriff

Joined: Oct 27, 2005
Posts: 19680
    
  18

It isn't. If the continue would be replaced by a break then they would have done the same thing, but the loop shouldn't end if the order is not processable.


SCJP 1.4 - SCJP 6 - SCWCD 5 - OCEEJBD 6
How To Ask Questions How To Answer Questions
conor o callaghan
Greenhorn

Joined: Oct 01, 2011
Posts: 5
Peter Kirk wrote:I'd probably just write it
for (cond) {
if (object.isProcessable()) {
other lines;
}
}

or a while loop as others have said, depending on what the iteration is. But I don't think
while (cond && object.isProcessable())

is neccessarily the same.



Its a simple for loop with a list of objects

I agree that the following is better

for (DataObject object : objects) {
if (object.isProcessable()) {
other lines;
}
}


I guess I just dont see the sense in. I saw this code and posted it here simply because
I prefer to avoid negated conditions.
A continue is not necessary and it feels like one is almost going out of there way to over complicate the code ( I know its not actually complicated)

I'm totally being anal about it but its not consistent with other existing coding practices. Its different for the sake of being different.
Ernest Friedman-Hill
author and iconoclast
Marshal

Joined: Jul 08, 2003
Posts: 24183
    
  34

I'll occasionally use the



format if indenting "something" one more level would be bad, or if "something" is complex and/or expected behavior. Most of the time I'd agree with your rewrite, but I wouldn't say that the original form is always out of place.


[Jess in Action][AskingGoodQuestions]
Peter Kirk
Greenhorn

Joined: Oct 12, 2011
Posts: 12
Recently I did actually do something similar, I must admit. I was bug fixing some code, and came across something like this


A problem could occur sometimes in the loop, where "object" could become invalid for further processing. So I just did something like:


so shoot me.
conor o callaghan
Greenhorn

Joined: Oct 01, 2011
Posts: 5
Ernest Friedman-Hill wrote:I'll occasionally use the



format if indenting "something" one more level would be bad, or if "something" is complex and/or expected behavior. Most of the time I'd agree with your rewrite, but I wouldn't say that the original form is always out of place.


yes but if its complex then shouldnt it be,

if( x )
{
doSomething();
}


As I said its a stupid post by me but I was curious as to what others thought. At the end of the day we are all individuals and we have our own preferences, but I guess our individuality can also cause inconsistency in the look and feel of the code. There is no right answers here but more so what is more acceptable... If you were to pick up the code from that person what would most people prefer..

I think we have our answer.
conor o callaghan
Greenhorn

Joined: Oct 01, 2011
Posts: 5
Peter Kirk wrote:Recently I did actually do something similar, I must admit. I was bug fixing some code, and came across something like this


A problem could occur sometimes in the loop, where "object" could become invalid for further processing. So I just did something like:


so shoot me.


Now in your circumstances I agree - I've done it many times simply because of the depth and other rules within the loop, the easies solution would be to continue or break out of it, but my problem with the code was that it was basic - There was no complexity, it was literally the for loop with about 4 lines of code being avoided by a continue..

There was no complexity no reason to do it in that way.

it was literally something like

for( Job job : jobs)
{
if( !job.isProcessable())
{
continue;
}

simple line of code no condition
simple line of code no condition
simple line of code no condition
simple line of code no condition
}
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7700
    
  20

Peter Kirk wrote:But I don't think
while (cond && object.isProcessable())
is neccessarily the same...

Actually, based on what we were supplied, it is; but now that conor has told us what the loop is actually doing, I think I'd write it as:
Winston

BTW - there's quite a good article on the lines of this subject here.
Peter Kirk
Greenhorn

Joined: Oct 12, 2011
Posts: 12
Winston Gutkowski wrote:
Peter Kirk wrote:But I don't think
while (cond && object.isProcessable())
is neccessarily the same...

Actually, based on what we were supplied, it is;


Possibly, but not "neccessarily". As you say, there wasn't entirely enough information if the original post.
Joe Areeda
Ranch Hand

Joined: Apr 15, 2011
Posts: 316
    
    2

I'll add my irrelevant opinion.

I would write it as:

I think

Is different because it stops at the first object that is not Processable and the original code searches the whole list.

I don't particularly object to negative conditions but I dislike continue, break and multiple returns when they are not necessary. It's an old "structured programming" concept that I internalized.
If functions an loops have multiple exit paths I believe they are more error prone.

Joe


It's not what your program can do, it's what your users do with the program.
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7700
    
  20

Joe Areeda wrote:I think
Is different because it stops at the first object that is not Processable and the original code searches the whole list.

At the risk of flogging this to death, the code originally posted wasn't clear and I took 'cond' to mean some kind of condition, rather than a for-each. And since the isProcessable() test was the first line in the loop, it seemed logical that it could be put outside.

Of course 'cond' could have included something to retrieve object, so I was wrong anyway; but at least it prompted further explanation .

Winston
Joe Areeda
Ranch Hand

Joined: Apr 15, 2011
Posts: 316
    
    2

Winston Gutkowski wrote:
Joe Areeda wrote:I think
Is different because it stops at the first object that is not Processable and the original code searches the whole list.

At the risk of flogging this to death, the code originally posted wasn't clear and I took 'cond' to mean some kind of condition, rather than a for-each. And since the isProcessable() test was the first line in the loop, it seemed logical that it could be put outside.

Of course 'cond' could have included something to retrieve object, so I was wrong anyway; but at least it prompted further explanation .

Winston
It is kind of unclear what is going on and there seem to be multiple interpretations so far.

The original code looks to me like a polling situation in a background task, where cond is some kind of switch to terminate the process and object.isProcesable() is somehow changing state external to the loop.

But who know what's really going on and whether the OP is really saturating that process waiting for one of those two thing to change.

Joe
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7700
    
  20

Joe Areeda wrote:It is kind of unclear what is going on and there seem to be multiple interpretations so far.
Actually OP has now clarified it as being basically a for-each loop.
The original code looks to me like a polling situation in a background task, where cond is some kind of switch to terminate the process and object.isProcesable() is somehow changing state external to the loop.
Wow, that never even occurred to me; but you're quite right. Yet another reason to read that "Tell, don't ask" article .

Winston
Wendy Gibbons
Bartender

Joined: Oct 21, 2008
Posts: 1107

the reason for the continue maybe becasue the "other code" is quite large, and the writer is trying to avoid too many indentations.
so something like



might be neater.

bother just read more of the thread, so will change it to say ... at one time before refactoring the other code may have been large
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Java Best Practice - Elegant code - Opinions please?