Java-Gaming.org    
Featured games (79)
games approved by the League of Dukes
Games in Showcase (477)
Games in Android Showcase (109)
games submitted by our members
Games in WIP (536)
games currently in development
News: Read the Java Gaming Resources, or peek at the official Java tutorials
 
    Home     Help   Search   Login   Register   
Pages: [1]
  ignore  |  Print  
  "Bad" programming?  (Read 5040 times)
0 Members and 1 Guest are viewing this topic.
Offline mmca

Senior Newbie




Java games rock!


« Posted 2005-03-24 11:09:34 »

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 == 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.

Thanks for any feedback.

Mick.
Offline kevglass

JGO Kernel


Medals: 122
Projects: 23
Exp: 18 years


Coder, Trainee Pixel Artist, Game Reviewer


« Reply #1 - Posted 2005-03-24 11:27:32 »

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.

Kev

Offline mmca

Senior Newbie




Java games rock!


« Reply #2 - Posted 2005-03-24 11:38:05 »

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.

Cheers for the help

Mick.
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline blahblahblahh

JGO Coder


Medals: 1


http://t-machine.org


« Reply #3 - Posted 2005-03-24 14:01:55 »

1. return from loop == very bad.

2. 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 Sad.

malloc will be first against the wall when the revolution comes...
Offline Abuse

JGO Coder


Medals: 11


falling into the abyss of reality


« Reply #4 - Posted 2005-03-24 15:26:05 »

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.

Quote

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 Sad.


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.

Make Elite IV:Dangerous happen! Pledge your backing at KICKSTARTER here! https://dl.dropbox.com/u/54785909/EliteIVsmaller.png
Offline MickeyB

Senior Member




my game will work, my game will work!


« Reply #5 - Posted 2005-03-24 18:26:04 »

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?

MickeyB

Current Project: http://www22.brinkster.com/mbowles/
Offline Raghar

Junior Member




Ue ni taete 'ru hitomi ni kono mi wa dou utsuru


« Reply #6 - Posted 2005-03-24 19:54:26 »

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.
Offline blahblahblahh

JGO Coder


Medals: 1


http://t-machine.org


« Reply #7 - Posted 2005-03-24 20:26:33 »

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.


And that is a problem because ... ?

malloc will be first against the wall when the revolution comes...
Offline kevglass

JGO Kernel


Medals: 122
Projects: 23
Exp: 18 years


Coder, Trainee Pixel Artist, Game Reviewer


« Reply #8 - Posted 2005-03-24 21:34:53 »

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).

Of course, it could be the beer talking Smiley

Kev

Offline K.I.L.E.R

Senior Member




Java games rock!


« Reply #9 - Posted 2005-03-25 00:02:17 »

Oh oh, I'm going to change the way I code now.
I use return statements inside loops when searching for something.

I do the checking before I return any value.

Vorax:
Is there a name for a "redneck" programmer?

Jeff:
Unemployed. Wink
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline Vorax

Senior Member


Projects: 1


System shutting down in 5..4..3...


« Reply #10 - Posted 2005-03-25 04:25:30 »

Quote
1. return from loop == very bad.


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 ... ?


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;

 char a1[] = s1.toCharArray();
 char a2[] = s2.toCharArray();

 for (int i=0; i < max; i++) {
   char c1 = a1;
   char c2 = a2;

   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.


To not hire someone for what is basically a matter of style and I would add practical experience, would be pretty weak.

Offline Abuse

JGO Coder


Medals: 11


falling into the abyss of reality


« Reply #11 - Posted 2005-03-25 08:26:28 »

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 Shocked

Make Elite IV:Dangerous happen! Pledge your backing at KICKSTARTER here! https://dl.dropbox.com/u/54785909/EliteIVsmaller.png
Offline blahblahblahh

JGO Coder


Medals: 1


http://t-machine.org


« Reply #12 - Posted 2005-03-25 10:16:00 »

Quote

When reading code, you  start at the top and work down.


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.

Quote

 The logic is instantly clear if you see a return in a loop.


The point is precisely that it is NEVER instantly clear to have your return anywhere but at the end of the method.

Quote

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.  


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.

Quote

So, you have complicated the code with the simplification.


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.

Quote

public int compareStrings(String s1, String s2) {
 int len1 = s1.length();
 int len2 = s2.length();
 int max = Math.min(len1, len2) + 1;
 // Set default value if no match found
 int result = len1 - len2;  // HERE

 char a1[] = s1.toCharArray();
 char a2[] = s2.toCharArray();

 for (int i=0; i < max; i++) {
   char c1 = a1;
   char c2 = a2;

   if (c1 != c2)
   {
     result = c1 - c2;  // HERE
     break; //HERE
    }
 }

 return result;
}

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.


Oh, really? You're sure about that?

Quote

To not hire someone for what is basically a matter of style and I would add practical experience, would be pretty weak.


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.

malloc will be first against the wall when the revolution comes...
Online EgonOlsen
« Reply #13 - Posted 2005-03-25 10:36:01 »

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...

Offline Vorax

Senior Member


Projects: 1


System shutting down in 5..4..3...


« Reply #14 - Posted 2005-03-25 14:38:36 »

Quote


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).


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.

Quote

EDIT: yeah, as Abuse points out, with your trivial example the problems of your approach don't really show up yet.


Looking at your code below, your fix succeeded in complicating it, re-stepped the logic and made it slower all at the same time Wink

Quote

Again, I often see the reverse.  Speaking striclty from experience here.


I am speaking strickly from experience as well.  Perhaps we just have different experiences.  

Quote

The point is precisely that it is NEVER instantly clear to have your return anywhere but at the end of the method.


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.

Quote

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.


I too speak from experience.

Quote

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.


We see things the opposite way, while both trying to reach the same goal Smiley

Quote

public int compareStrings(String s1, String s2) {
 int len1 = s1.length();
 int len2 = s2.length();
 int max = Math.min(len1, len2) + 1;
 // Set default value if no match found
 int result = len1 - len2;  // HERE

 char a1[] = s1.toCharArray();
 char a2[] = s2.toCharArray();

 for (int i=0; i < max; i++) {
   char c1 = a1;
   char c2 = a2;

   if (c1 != c2)
   {
result = c1 - c2;  // HERE
break; //HERE
    }
 }

 return result;
}

Oh, really? You're sure about that?


Absolutley.  You just proved it.

Quote
 // Set default value if no match found
 int result = len1 - len2;  // HERE


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.  


Quote

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.


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.

Offline mmca

Senior Newbie




Java games rock!


« Reply #15 - Posted 2005-03-25 16:00:37 »

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 == true)
          found = true;
     else
          i++;
 }
 if(found)
    return i;
 else
    return -1;
}

Thanks for the info on it all though.
Mick.
Offline blahblahblahh

JGO Coder


Medals: 1


http://t-machine.org


« Reply #16 - Posted 2005-03-25 16:53:34 »

Quote

Looking at your code below, your fix succeeded in complicating it, re-stepped the logic and made it slower all at the same time Wink


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.

malloc will be first against the wall when the revolution comes...
Offline Vorax

Senior Member


Projects: 1


System shutting down in 5..4..3...


« Reply #17 - Posted 2005-03-25 19:26:29 »

Quote


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?


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.

Quote

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.


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.


Quote

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.


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 Wink


Offline Raghar

Junior Member




Ue ni taete 'ru hitomi ni kono mi wa dou utsuru


« Reply #18 - Posted 2005-03-25 20:53:11 »

Robust way could be something like this.

public int min(){
 final int max = maxSizeOfThisSet < numbers.length ? maxSizeOfThisSet : numbers.length;
if(max == 0){return -1;}
int min1 = numbers[0];
 for(int c1 = 1; c1 < max; c1++){
     if(numbers[c1] < min1) {
          min1 = numbers[c1];}}

return min1;
}  


People that are against multiple return statements in a method should do more ASM programming, and don't try Common LISP like behaviour.
Offline swpalmer

JGO Coder




Where's the Kaboom?


« Reply #19 - Posted 2005-03-26 03:25:47 »

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?
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
// 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.:
1  
2  
3  
4  
5  
6  
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...  Smiley)
'cause the code formatting on these forums stinks!

Offline Vorax

Senior Member


Projects: 1


System shutting down in 5..4..3...


« Reply #20 - Posted 2005-03-26 16:50:15 »

Quote
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.


I am hiring 1 more Java dev http://www.java-gaming.org/cgi-bin/JGNetForums/YaBB.cgi?board=jobs;action=display;num=1109811231

and I won't hold it against you if you use more then one return in a method Wink

I know this doesn't really relate, but I am getting desperate, the Java market in Ottawa is saturated Sad

Pages: [1]
  ignore  |  Print  
 
 
You cannot reply to this message, because it is very, very old.

 

Add your game by posting it in the WIP section,
or publish it in Showcase.

The first screenshot will be displayed as a thumbnail.

CogWheelz (18 views)
2014-07-30 21:08:39

Riven (25 views)
2014-07-29 18:09:19

Riven (15 views)
2014-07-29 18:08:52

Dwinin (12 views)
2014-07-29 10:59:34

E.R. Fleming (33 views)
2014-07-29 03:07:13

E.R. Fleming (12 views)
2014-07-29 03:06:25

pw (43 views)
2014-07-24 01:59:36

Riven (43 views)
2014-07-23 21:16:32

Riven (30 views)
2014-07-23 21:07:15

Riven (31 views)
2014-07-23 20:56:16
List of Learning Resources
by SilverTiger
2014-07-31 18:29:50

List of Learning Resources
by SilverTiger
2014-07-31 18:26:06

List of Learning Resources
by SilverTiger
2014-07-31 13:54:12

HotSpot Options
by dleskov
2014-07-08 03:59:08

Java and Game Development Tutorials
by SwordsMiner
2014-06-14 00:58:24

Java and Game Development Tutorials
by SwordsMiner
2014-06-14 00:47:22

How do I start Java Game Development?
by ra4king
2014-05-17 11:13:37

HotSpot Options
by Roquen
2014-05-15 09:59:54
java-gaming.org is not responsible for the content posted by its members, including references to external websites, and other references that may or may not have a relation with our primarily gaming and game production oriented community. inquiries and complaints can be sent via email to the info‑account of the company managing the website of java‑gaming.org
Powered by MySQL Powered by PHP Powered by SMF 1.1.18 | SMF © 2013, Simple Machines | Managed by Enhanced Four Valid XHTML 1.0! Valid CSS!