This week's book giveaway is in the Cloud/Virtualizaton forum.We're giving away four copies of Mesos in Action and have Roger Ignazio on-line!See this thread for details.
Win a copy of Mesos in Action this week in the Cloud/Virtualizaton forum!

# loop problems

Raine RaineAndrews
Greenhorn
Posts: 9
I'm working on some new code to practice on loops and remainders (and thus divisors). I finished the darn thing, and I can follow it to the very end, but it seems to be stuck somewhere in the loop, since it never outputs the data after compiling an running the program.

My naming is atrocious, i know! Hopefully the //comments can help lead you to the problem though!

Also, if you know a way you can make this more elegant, for example, using if else, if then, or for. Feel free to point it out. Right now its all a blur to me!

Sridhar Santhanakrishnan
Ranch Hand
Posts: 317
A couple of things

try using if-then. A while condition runs a loop which is not needed at some points of your code.

the semicolon at the end is making the code go into an infinite loop.

Raine RaineAndrews
Greenhorn
Posts: 9
Thanks! Deleting the semicolon and adding a { didn't seem to help, but I followed your advice on using if and else statements. I actually got output! Unfortunately, my output was 1 with 1 divisor, which can't be right. I'm betting that I need an Else statement in there somewhere right?

I had to change the booleon regarding divisors (line 18) to not accept 0 because it was trying to divide by zero for some reason. I think by fixing the symptom and not the problem may have ruined the process however.

No piece of advice is too small, I'm just starting and I'm self teaching through doing. It gives me headaches, and it takes forever to learn how to put together simple code, but I'm a heck of a lot further than I would be reading instead of doing!

Here's the code that feeds me 1 with 1 divisor

Here is the code where I take a stab at the else statement...

Thanks again for any help. This site is really Greenhorn friendly (go figure!).

Sridhar Santhanakrishnan
Ranch Hand
Posts: 317

Why are you setting Divisors back to 0? You check that Divisors is greater than zero. Think what would happen for the next iteration of the loop(TestNumber =2).

Plus, its enough if you loop until Divisors is less than TestNumber (or TestNumber/2). Any number greater than TestNumber cannot be a proper divisor.

Raine RaineAndrews
Greenhorn
Posts: 9
That's why it spit "can't divide by zero" at me! I'll change it to =1 and get rid of that nasty looking booleon to boot! And if I cut down the amount of calculations by restricting the while loop for divisors, I get a more elegant piece of code

I also figured out I don't need an else statement, and that Divisors++ shouldn't be part of the if statement, but part of the While loop for divisors. What a simple mistake, with such a big consequence!

Here's the finished code!

Thanks for the help, I learned a lot about Java loops on this program!

David Newton
Author
Rancher
Posts: 12617
On a stylistic note, it's conventional to start the name of non-final variables with a lower-case letter. Doing so makes it easier for other people to read and understand your code more quickly. It's a bit over-commented, too; it's really too much to read and comprehend easily.

I'm also not a fan (at *all*) of declaring the variables in one place and initializing them in another; this is a holdover from the good ol' days of C when you *had* to declare all your local variables at the beginning of a method. Now it just makes things confusing by adding that much more cognitive overhead.

Also, classes and methods should be documented with Javadoc comments (when necessary) to allow the standard Javadoc tool to create API documentation.

I actually had to read the code a couple of times; the variables aren't always named with (with what I consider to be) something very clear:

- currentIntMax: it's really the "numberWithMaxDivisors", and named to match the "maxDivisors" variable
- divisors: it's not a plural anything, it's really the "currentDenominator"
- testNumber: might as well just call it "numerator"
- maxDivisors: the comment is actually misleading; it's actually a reasonable name, but the comment is wrong.

Note also how a good naming policy makes the initialization comments go away, because they'd be redundant.

I'm not sure you check divisors all the way up to and including the current numerator--a number can't be evenly divided by a different number greater than half of itself, correct?

I'm not convinced a while loop is the clearest way to express the intent of the loops; in order to fully understand the nature of the loop you have to read all the way through it to understand when and why the counter variable is modified. A for loop puts all that information at the top of the loop (assuming we're not doing something tricky and modifying the counter in the loop body).

And remember--not everyone that runs your program will be running it from DOS; making OS-specific comments rarely makes sense unless you're specifically targeting only a specific OS, which is relatively rare in the Java world.

Just some good habits to start early... while I almost never post complete code examples, you've already solved the problem: here's what it looks like with some of the above changes in place. See what you think--my coding style doesn't work for everybody, but it's sometimes nice to see alternatives.