This week's book giveaways are in the Java EE and JavaScript forums.
We're giving away four copies each of The Java EE 7 Tutorial Volume 1 or Volume 2(winners choice) and jQuery UI in Action and have the authors on-line!
See this thread and this one for details.
The moose likes Beginning Java and the fly likes I got it, but is my code sloppy? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of The Java EE 7 Tutorial Volume 1 or Volume 2 this week in the Java EE forum
or jQuery UI in Action in the JavaScript forum!
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "I got it, but is my code sloppy?" Watch "I got it, but is my code sloppy?" New topic
Author

I got it, but is my code sloppy?

Ryan Smith
Ranch Hand

Joined: Jun 29, 2004
Posts: 40
What do you guys think of this code to find the lowest common multiple (LCM):
Greg Charles
Sheriff

Joined: Oct 01, 2001
Posts: 2849
    
  11

Not too bad! You have Javadoc style comments inside the method, which you don't need. They won't show up in the Javadocs anyway. Maybe rename d to something more descriptive? That's being a bit picky. Style-wise everything else looks fine. I think the algorithm is wrong though. You don't mean to divide both x and y by the common factor, do you? That would result in too low of an answer.
Tim West
Ranch Hand

Joined: Mar 15, 2004
Posts: 539
Looks good to me also. If it was my code, I'd use longer variable names (probably...in math-heavy code I don't always ). Also, I'd move the "final possibility" clause to the very start - this is much more efficient, you can avoid the main body algorithm.

Finally, I'd consider using Euclid's Algorithm for finding the LCM. It's very efficient, and not too complex. Dunno if efficiency is a concern though.



--Tim
[ June 30, 2004: Message edited by: Tim West ]
Greg Charles
Sheriff

Joined: Oct 01, 2001
Posts: 2849
    
  11

No, I'm wrong. You do need to divide both x and y by the common factor. However, you need to keep track of the common factors. I think this works:



It's a good idea to avoid nested loops if you can. Also, the less you use break and continue, the better!
Mala Gupta
Author
Ranch Hand

Joined: Sep 27, 2002
Posts: 251
    
    9
Hi Greg

> It's a good idea to avoid nested loops
> if you can. Also, the less you use break
> and continue, the better

What are the disadvantages of using nested loops, continue and break.

regards
Mala


Author of Manning's OCA Java SE 7 Programmer I Certification Guide and OCP Java SE 7 Programmer II Certification Guide
Nick Knickerbocker
Greenhorn

Joined: Jan 14, 2002
Posts: 3
Ryan, Looking good man! Greg had good suggestions about the comment type and variable name. Use inline comments within methods. Use descriptive variable names. For other good tips on Java coding style, I recommend the book Elements of Java Style (Vermeulen et al.).

Mala, as with most style guidelines, Greg's suggestions are based on code clarity and readability. As software developers, one of our primary concerns is how well our code communicates its intention. Nested loops are by nature more difficult to comprehend. Sometimes break and continue are useful, but one should take care to make them obvious in code because they violate the normal flow of looping control structures.

Greg, your version of the LCM algorithm looks great, but upon cursory inspection I would have to delete that second-to-last line. Am I wrong?


Cheers to all,
Nick
Jeroen Wenting
Ranch Hand

Joined: Oct 12, 2000
Posts: 5093
Originally posted by Mala Gupta:
Hi Greg

> It's a good idea to avoid nested loops
> if you can. Also, the less you use break
> and continue, the better

What are the disadvantages of using nested loops, continue and break.


readability. Code with deep nested loops and many breaks and continues can be hard to comprehend and therefore hard to change/maintain.


42
Ryan Smith
Ranch Hand

Joined: Jun 29, 2004
Posts: 40
Hey guys, thanks for all the great suggestions!! I knew that nested loops looked ugly, but I wasn't sure what was wrong...it just looked funny to me.

So you guys are saying comments in /***/ style only appear if written before a mehtod is declared, right? And inside a method, only use // ?
Angel Dobbs-Sciortino
Ranch Hand

Joined: Sep 10, 2003
Posts: 101
/** */ is generally used for javadoc, so you can use // if you have short comments within the method. You can also use /* */ if the comments are long.
Greg Charles
Sheriff

Joined: Oct 01, 2001
Posts: 2849
    
  11

Originally posted by Nick Knickerbocker:
Greg, your version of the LCM algorithm looks great, but upon cursory inspection I would have to delete that second-to-last line. Am I wrong?


Nick


Yes, you are wrong. The loop only makes x and y relatively prime. That is, it pulls out their common factors and stores them in a running tally. (No, tally's not the right word. Running product?) If x and y started relatively prime, for example 4 and 15, the running product after the loop would be 1, and x and y still the same. Ah, but maybe you were thinking of greatest common factor?
James Chegwidden
Author
Ranch Hand

Joined: Oct 06, 2002
Posts: 201
GCD:


LCM:

This is the way I would do it!!


Author and Instructor, my book
 
It is sorta covered in the JavaRanch Style Guide.
 
subject: I got it, but is my code sloppy?