Freezing While Loop in Java

[a_giunta67]

First off, I'm a complete noob at programming. I'm taking a Java class in school and I can't figure out why this loop freezes. The assignment is to make a Craps game using the Objectdraw library.

import java.io.*;
import objectdraw.*;

public class craps extends WindowController{
	private RandomIntGenerator die = new RandomIntGenerator(1, 6);
	
	int roll1;
	int roll2;
	int userPoint;
	int rollTotal;
	int reRollTotal;
	boolean firstRoll;
	boolean reRoll;


	 
	public void begin(){
		setSize(500,500);
		new Text("Click to roll.", 10, 20, canvas);
		roll1 = die.nextValue();
		roll2 = die.nextValue(); 
		rollTotal = (roll1+roll2);
		}

	
	
	public void onMousePress(){
		firstRoll = true;
		reRoll = false;
		
		if(firstRoll == true){
		canvas.clear();
	new Text("You rolled a " + (rollTotal),10,20,canvas);
	 	if(rollTotal == 7 || rollTotal == 11){
	 		new Text("You win!", 10, 40, canvas);
	 		}
	 
	 		else if(rollTotal == 2 || rollTotal == 3 || rollTotal == 12){
	 			new Text("You've crapped out.", 10, 40, canvas);
	 			firstRoll = false;
	 			reRoll = false; 
	 			}
	 	
	 		else{
	 			new Text("You didn't win or crap out.  Your point is " + rollTotal + ". \nRoll again.", 10, 40, canvas);
	 			userPoint = rollTotal;
	 			reRoll = true;
	 			firstRoll = false;
	 			}
	 			
	 		while(!firstRoll){
		 		reRollTotal = roll1+roll2;
		 		new Text(reRollTotal, 10, 80, canvas);
		 		}
	 	}
 		
	 	

	 	
	 	
	 	}
}

 

[berry120]

Well, let's take a look at the body of the while loop and look at exactly what it's doing:

while(!firstRoll){
    reRollTotal = roll1+roll2;
    new Text(reRollTotal, 10, 80, canvas);
}

So, if your firstRoll boolean is set to false, you add roll1 and roll2, assign them to reRollTotal and then create some new text with this total. The while loop then checks again whether firstRoll is false, it still is (because nothing in the loop changes the value of firstRoll) so it does exactly the same thing again. And again. And again. Giving you an infinite loop and therefore causing the thing to lock up.

At a quick glance it looks like your while loop should be an if statement - that way it'll only execute once each time you press the mouse, which is probably what you want.

A couple of other general pointers while I'm at it:

- When submitting code in code blocks, please try to indent it nicely - huge tabs everywhere make things a bit difficult to read! 4 spaces in place of a tab is a better alternative.

- It's very bad practice to have fields as anything other than private in Java (with the exception of constants, i.e. static final.) You've done it for your top field, but the rest should all be private also. If you want to know why it's a good habit to get into, just google around or ask and I'll elaborate.

- Are you sure you need your io import? I may be wrong because I haven't tried to compile your code, but I can't see anything from the io package used anywhere?

- Class names should ALWAYS (no exceptions here) start with a capital letter.

- The first line of your onMousePress class is:

firstRoll = true;

This is almost certainly wrong - otherwise whenever you press the mouse firstRoll will be set to true. You probably want it in your begin method instead.

- Whilst not strictly wrong, statements like this:

if(firstRoll == true)

...are a bit long winded. The quicker and more preferred way of writing it is simply:

if(firstRoll)

There's at least a couple of other bugs I think I've spotted, but I don't want to do the whole lot for you - try fixing those pointers and then feel free to ask again if you get stuck.

 

[a_giunta67]

Thank you so much! I got my loop to work, but I'm stuck on two last things.

1) I want it so that once the person wins or loses on the re-roll, the canvas clears and goes back to the beginning when it says "Click to Roll".
I had this:

while(!firstRoll){
   roll1 = die.nextValue();
   roll2 = die.nextValue();
   reRollTotal = roll1+roll2;
      if(reRollTotal == userPoint){
         new Text("Re-roll total = " + reRollTotal + "You've won!,10,60,canvas);                
      }
      else if(reRollTotal == 7){
	 new Text("Re-roll total = " + reRollTotal + " You lost.",10,60,canvas);
      }
      else {
	   new Text("Re-roll total = " + reRollTotal + " Click to Roll Again",10,60,canvas);
      }
		 		
firstRoll = true;
}

2) When the person wins or loses on the first roll, they should be able to to click again and have the program start over.

 

[berry120]

Your change to the loop is correct in the sense that it will now only execute once, but it will now only ever execute once since you always set firstRoll to true at the end of the iteration! It makes no sense to use a while loop there - you really should use an if statement instead.

Taking your problems again:

1) You could accomplish this by having a "reset" flag or similar - if you win or lose the game, the flag is set to true. You'd also have another check at the start of your onMousePress() method to check whether this flag is true - if it is, you set it to false and reset the canvas (this would be best accomplished by creating a separate method called reset, and calling that to avoid code duplication.) If your flag is false, you carry on with the method as normal.

2) Again, you've got the bit of code that deals with winning on the first roll already - use the same reset flag as you coded in 1) and set it to true if the player wins or loses.