Im doing a thing for colleage to return the first element of a set, but were off for Easter so I can’t ask if this is “bad” programming. Should I break out of a for loop like this or is this bad programming?
public int min()//returns the smallest value in the Set or returns -1 if the set is empty
{
for(int i = 0; i <maxSizeOfThisSet; i++)
{
if(numbers[i] == true)
return i;
}
return -1;
}
One other question is in a quiz game I’m building one of the main procedures called is recursive, but I could easily make it iterative. I’m trying to make it as fast as I can, and I was wondering whether Java has larger procedure overheads or whether it is generally faster to use assignment statements etc.
Returning out of the middle of loop is largely a matter of taste. It makes the code slightly harder to follow in my opinion but I don’t believe there is any practical reason to worry about it these days (not like in K&R C days).
Re: Recusion vs Iterative loop. Some work has been done in making the compiler sort this thing out for you but I’m not sure whether its actually available as yet. I would suspect it would be to do with the number of local variables and parameters you have in your procedure. The more the large the stack addition each loop (and hence the more that has to be stored/retrieved). Best bet as always is to profile it.
Thanks for the quick reply, just thought the jumping out of the middle of a loop reminded me of something we were told not to do when we werer learning PASCAL. I try to bench most the code, seems its faster using the iterative option, probably cause I’m passing a couple of pretty big objects as parameters.
break from loop == either “good” or “bad” depending on your perspective
1 is bad because one of the few things that people absolutely expect is that the last thing that happens in a method source code is the return. This doesn’t have to be the case, and often isn’t, but nearly always is. And the fact that the start is ALWAYS (and, in java, MUST be, unlike some other languages) the start of the source makes it easier if the last line is the return.
There are lots and lots of times in OOP where you want to modify, check, or log a return value before returning it. It’s much much harder to modify code to do this if you are returning from multiple places in the method.
Typically, if you find that returning from the middle is needed because the method is too huge and complex…you need to split into more methods.
So, instead, you break or jump.
re: 2, personally I think it’s a good thing so long as dropping out the bottom of the loop by end-of-iteration and by breaking both are correct and leave results in correct state IMMEDIATELY you leave the loop (i.e. you don’t have to do some fixup code after the loop depending upon how it was left). But that’s personal preference and may be bad for some reason :(.
In my experience, having multiple returns in a single method makes for less maintainable code. Its the closest thing Java has to goto.
Its also 1 of the warning signs we look for when evaluating sample source code from any job applicants.
yeah, I agree with that.
There are many situations where breaking from a loop is not only more efficient, but actually looks more elegant too.
Others in my office don’t though, they’re of the strict opinion that a while(condition) loop should only break when the condition is met - regardless of how elegant a solution with breaks is.
I can however see why they hold that point of view.
In the same way that returns mid-method force you to scan over the entire method to find all the possible exit paths, a break mid-loop does the same.
However, while a perfectly well structured method may span several pages of code (and hence be very hard to find all the exit paths) - a well structured loop should be a great deal more compact, therefor making any breaks alot easier to spot.
Interesting. I see returns in loops all over the place. Anyway, you are suggesting setting a variable, then break, then have the return(that set variable) at the end of the method?
Some of my methods are simplier and faster because they have multiple returns.
If I would insist on single return, I would need to add one control variable, or it will not be possible by easy code at all.
[quote]Some of my methods are simplier and faster because they have multiple returns.
If I would insist on single return, I would need to add one control variable, or it will not be possible by easy code at all.
[/quote]
And that is a problem because … ?
The strongest thing above that makes returns from the inside of a loop stated above is that it makes it hard to read. Well, whooptee doo… if this is the biggest thing you’ve got problems with while developing you’re doing pretty well.
I totally agree its not desirable, but its really a pretty minor offense over all. And choosing programmers based on such a minor point really is a bit petty (obviously IMO).
[quote] 1. return from loop == very bad.
[/quote]
I disagree. It depends on the situation. Returning from a loop in many cases simplifies the code and makes it easier to read.
When reading code, you start at the top and work down. The logic is instantly clear if you see a return in a loop. You know there is no point in progressing further because an answer has been reached. If a flag or value is set, then returned at the bottom of the method, you have to follow it further to complete that path becuase you don’t know if there are other operations that may be applied or conditions that are tested. So, you have complicated the code with the simplification. In other situations, you are truly adding complexity by introducing code where it is not only unecessary, it’s bad and can lead to errors.
[quote] And that is a problem because … ?
[/quote]
Here is a classic example - lexicographical comparisons (similiar situations arrise when doing other types of comparisons, assignments and traversals using arrays):
/** A lexicographic string comparision.
*
Returns:
A Negative number: s1 lexicographically precedes s2
A Positive number: s1 lexicographically follows s2
0 if they are equal
*/
public int compareStrings(String s1, String s2) {
int len1 = s1.length();
int len2 = s2.length();
int max = Math.min(len1, len2) + 1;
for (int i=0; i < max; i++) {
char c1 = a1[i];
char c2 = a2[i];
if (c1 != c2)
return c1 - c2;
}
return len1 - len2;
}
If that method were broken out into the values which it self resolves, it would be much more difficult to read and worse, it’s a target for potential bugs. In this code, three possibilities are covered by two return statements, without the addition of any other variables, if conditions and/or one or more breaks.
[quote] Its also 1 of the warning signs we look for when evaluating sample source code from any job applicants.
[/quote]
To not hire someone for what is basically a matter of style and I would add practical experience, would be pretty weak.
Perhaps I should give a context on which my statement was based
Imagine a paint(…) method that is 4000 lines long and is littered with 25 return statements.
Imagine then trying to add a little debug drawString to the end >_<
Its an all too common sight with the shit J2ME games I have to port.
Your lexicographic string comparision example is an obvious case where clarity is not lost by returning early, primarily because its a tiny method :o
Only for trivial code or if you’re doing time-intensive line-by-line debugging - and when writing trivial code, nothing matters because none of the problems of bad programming show up. The reason we do things a certain way is to avoid problems when the code becomes long and convoluted (incidentally - the raison d’etre for OOP).
EDIT: yeah, as Abuse points out, with your trivial example the problems of your approach don’t really show up yet.
The point is precisely that it is NEVER instantly clear to have your return anywhere but at the end of the method.
And in the majority of cases at some point in the future someone will have to modify that source and add “other operations” anyway. I speak from long experience. Again, this is part of why we bother with the additional effort and inconvenience of OOP: not to save time NOW but to save it LATER.
Sure, if you thought it was a simplification of raw source then I would agree it’s a bad one. Rather, it’s a simplification of maintenance effort.
Oh, really? You’re sure about that?
The coding style you propose is responsible for costing or saving thousands of dollars on individual projects even with only a few programmers. In general, bad coding style can burn as much as 30% of the total project cost. Go figure.
Like with any other coding style, there are places where it’s reasonable and places where it’s not. Saying that it’s bad and giving a 4000 line method as an example is questionable IMO. Coding style is (to a large extend) a matter of taste…we have people at work, who clutter the whole source with finals to prevent themselfs from changing the values later in the code by accident (Something that i may have done only 10 times in 20 years). They argue that it’s the greatest thing since bread being sliced, i hate it with a passion. But the same people who are calling this a good style are constantly using classes from sun.*, which i consider to be very bad. Or they put the opening brackets in the next line or are mixing english and german words for the naming of methods or or or…
To me, most of this is a matter of taste and so is returning from anywhere within a method in most cases. It may not be suitable to do it all over the place in a 4000 line method but it is in a 10 line method. Do what fits your needs…
Again, i disagree. In trivial code, your belief holds truer but is still usually false. As code becomes more complicated there is more and more to follow. I am well aware why it is claimed to be the way to do things, but more often then not, it just complicates it.
Looking at your code below, your fix succeeded in complicating it, re-stepped the logic and made it slower all at the same time
I am speaking strickly from experience as well. Perhaps we just have different experiences.
But it is clearer. The only thing having a return at the end of the method tells you is : “If I follow this code through all paths I will end up here some how”. Where as having a return earlier means “I have followed the code and now the remainder does not apply to this condition”. When debuging code, that piece of knowledge can save you time. It also tells you that if you don’t think the path is truly complete, then this is a potential bug. If a variable was assigned, then you would have to follow it down to the return at the bottom to check for other assignments.
I too speak from experience.
We see things the opposite way, while both trying to reach the same goal
Absolutley. You just proved it.
[quote] // Set default value if no match found
int result = len1 - len2; // HERE
[/quote]
You just handled the exception condition first. Logic would indicate a default value should fulfill the most common condition, not the exception. Also, because it is the exception, it is wasteful to determine it at this time and therefore less efficient. When debugging, this code has added complexity because you later modify the value. “int result” may NOT be the real result. The maintainer will have to follow it all the way through now to see if it will be modified somwhere else (and it may be in this case). As the complexity and size of the method grows, this will get harder and harder to do. There is no value added by that change. It is more complicated, less efficient and harder to debug.
I agree about bad coding style, but I don’t see this as an example of that. If it keeps the logic clearer and makes the code simpler to read, and it’s more efficient, then I find little problem with it, despite the “rule of thumb”. I have dozens of programmers working for me and I wouldn’t fault any of them for doing a return in a loop, unless it was a case where it made things less clear, and those IME are the exception.
This has gone off in a direction which I wouldn’t insult you all with, by giving my newbie opinions, but as far as my very simple method is concerned I changed to a while, just to save any trouble. Generally we have been taught to not do it, but I’ll leave it to yourselves to decided whether we should or not.
public int min()//returns the smallest value in the Set or returns -1 if the set is empty
{
int i =0;
boolean found = false;
while(found!=true && i <maxSizeOfThisSet)
{
if(numbers[i] == true)
found = true;
else
i++;
}
if(found)
return i;
else
return -1;
}
I dont have your docs
I dont have your use cases
In short, I dont have your SRS
How the heck am I supposed to know what aspects of your code, without affecting the final calculated result, are required and which are not?
I only made the modification to point out how simple the transformation is, and how little effort it takes. If you are really concerned that the logic has changed then I suggest you don’t use java, because the runtime + compiler are guaranteed to make many more changes than I just did. If the program runs the same for the same inputs, then they are legal (unless Sun’s taken a new “lets’ keep java slow” initiative since I last checked and is using the more strict form of optimizations).
Your claims about efficiency are specious; the most basic of compilers will detect and correct the things you mentioned IFF they were actually bad for performance for the current platform, and of course they might always be good instead. Part of why I love java: I no longer have to waste time continually re-factoring code in ways that doesn’t change the results but is necessary for “performance”, and requires lots of copy+paste+re-order args +refactor signatures etc.
Hehe. Neither do I, but the lexicographical algorithm is pretty standard, I just wrote a quick implementtion because it is a good example of a good reason to use a return within a loop.
But developers don’t have to read byte code, which was the point of the discussion. The change may have been easy to make, but it added no value and instead makes the logic harder to follow, decreased efficiency and would require more effort to debug. All only slightly becuase it is a very small example, but the problem scales up with the code.
Again, I am not concerned with the byte code, only the readable code. The efficiency lost was another assignment and a subtraction operation which would not be necessary in the majority of comparisons. No biggies. I was only illustrating the point.
I don’t think I can convince you that a returning in a loop is not necessarily bad, and I can guarantee you can’t convince me it’s bad practice…so, let’s call this a YARD (Yet Another Religous Debate) problem and allow each other to continue to worship his own deities
If your methods are so long that you can’t see that there is a conditional return somewhere in the middle you have other problems.
Returning always at the end of a method often means introducing more compexity into the method to handle different exit conditions properly - in such cases the added complexity can be worse than the chance of not spotting a return in the middle when glancing at the code.
BTW…
Is this any better?
// assumes MAX_VALUE will not occur in numbers
public int min(){
final int max = Math.min(maxSizeOfThisSet , numbers.length);
int min1 = Integer.MAX_VALUE;
for(int c1 = 0; c1 < max; c1++){
if(numbers[c1] < min1) {
min1 = numbers[c1];
}
}
return min1 == Integer.MAX_VALUE ? -1 : min1;
}
I often put quick exit checks at the very TOP of mehtods.
e.g.:
public int min() {
if (set.isEmpty())
return -1;
...
}
When are we getting those new forums? (The ones that were coming before the end of last year… then sometime in January. You know the ones that were ready to go months ago and we never heard about it again… :))
'cause the code formatting on these forums stinks!
