Beginner's Yahtzee program

[Jon Davis]

I am in the process of teaching myself how to program in Java, and I was wondering if you guys could look over my code and give me some help with how I am doing? I am really struggling with the methods aspect of programming. If you could point out any recommendations to make my program better and easier to follow it would be appreciated. This is what I have so far, I haven't got very far but before I get too far along, I would like to know if there is something I should be doing differently.

package yahtzee;
/*
 Author: Jon Davis
 File: Yahtzee.java
 Date: 2013 Oct 27
 Description: This program is simple yahtzee program
 that runs in the console 
 */
import java.util.Scanner;
import java.util.Random;

public class Yahtzee
{
	private static final Random randomNumber = new Random();
	private static final Scanner input = new Scanner( System.in );
	public static int s1, s2, s3, s4, s5; //declaring the variables that will be used in the
									      //selecting which dice to re-roll
	public static int rollPerTurn; //declaring variable to keep track of rolls per turn
	private static int [] dice = new int [5]; //declaring an int array representing the 5 dice
	public static void main( String args[] )
	{
		while (true){
		//creating a menu
		System.out.println("********Menu********");
		System.out.println("1. Play Yahtzee");
		System.out.println("2. Help");
		System.out.println("3. Quit");
		System.out.println("********************");
		int selection = input.nextInt();
		if ( selection == 1){
			firstRoll();
			break; //if 1 is selected start game and end the menu
		}
		else if ( selection == 2){
			yahtzeeHelp(); // if 2 display "help" screen and go back to menu
		}
		else if ( selection == 3 ){
			break; //end game
		}
		else{
			System.out.println("No such selection");
		}
		}
		while ( rollPerTurn < 3 ){
		System.out.println();
		s1 = input.nextInt();//obtaining input to determine which dice to re-roll
		s2 = input.nextInt();
		s3 = input.nextInt();
		s4 = input.nextInt();
		s5 = input.nextInt();
		reRoll();
		}
	}
	
	public static int firstRoll() //method to roll 5 dice
	{
		for ( int i = 0; i < dice.length; i++){
			dice[i] = rollDice();
		}
		System.out.println("Select which dice to reroll.");
		for ( int a = 0; a < dice.length; a++ ){
			System.out.print(dice[a] + " " );
		}
		rollPerTurn = 1; //indicating this is the first roll
		return rollPerTurn;
	}
	public static int [] reRoll() //method to re-roll certain dice
	{
		if ( s1 == 1 ){
			dice[0] = rollDice(); //re-roll Die 1
		}if ( s2 == 1 ){
			dice[1] = rollDice(); //re-roll Die 2
		}if ( s3 == 1 ){
			dice[2] = rollDice(); //re-roll Die 3
		}if ( s4 == 1 ){
			dice[3] = rollDice(); //re-roll Die 4
		}if ( s5 == 1 ){
			dice[4] = rollDice(); //re-roll Die 5
		}
		rollPerTurn++;
		for ( int b = 0; b < dice.length; b++ ){
			System.out.print(dice[b] + " " ); //displaying the dice after re-roll
		}
		return dice;
	}
	public static int rollDice() //method to roll a die
	{
		int die = 1 + randomNumber.nextInt( 6 );
		return die;
	}
	public static void yahtzeeHelp() //the "help" screen
	{
		System.out.println("Your dice will show up and you will input a 1 or a 0");
		System.out.println("under each die.  A 1 indicates you wish to re-roll");
		System.out.println("that particular die, and a 0 means you wish to keep it.");
	}
}

Thank you

[Jam Rei]

In the code above is better to use switch case statement instead of a lot of if statements.

[Jon Davis]

Sveto Koychev wrote:In the code above is better to use switch case statement instead of a lot of if statements.

Can I use a switch statement for choosing which dice to re-roll. I replaced the menu portion of the code with a switch statement, but I am unsure how to replace the code in the re-roll method because it deals with multiple variables. I also made the menu a method because I am trying to get more practice using methods. I really appreciate your feedback.

	public static void menu()
	{
		System.out.println("********Menu********");
				System.out.println("1. Play Yahtzee");
				System.out.println("2. Help");
				System.out.println("3. Quit");
				System.out.println("********************");
				int selection = input.nextInt();
				switch ( selection )
				{
					case 1:
						firstRoll();
						break;
					case 2:
						yahtzeeHelp();
						break;
					case 3:
						exit();
						break;
					default:
						System.out.println( "No such selection." );
		}

Below is the part I am unsure about how to use a switch statement.

public static int [] reRoll() //method to re-roll certain dice
	{
		if ( s1 == 1 ){
			dice[0] = rollDice(); //re-roll Die 1
		}
		if ( s2 == 1 ){
			dice[1] = rollDice(); //re-roll Die 2
		}
		if ( s3 == 1 ){
			dice[2] = rollDice(); //re-roll Die 3
		}
		if ( s4 == 1 ){
			dice[3] = rollDice(); //re-roll Die 4
		}
		if ( s5 == 1 ){
			dice[4] = rollDice(); //re-roll Die 5
		}
		rollPerTurn++;
		for ( int b = 0; b < dice.length; b++ ){
			System.out.print(dice[b] + " " ); //displaying the dice after re-roll
		}
		return dice;
	}

[Campbell Ritchie]

Do you have an array of 5 dice? In that case you should use the number to calculate the index in the array:- numbers[i] = j;
You would not want a switch‑case at all. In the instance you showed, you would probably need i - 1 rather than i

Why do you have so many things marked static? That usually denotes non‑object‑oriented design.

And welcome to the Ranch

[Jon Davis]

Thank you for your feedback. I do have an array for the 5 dice, however I am not using it yet. As far as having so many things marked as static its because I kept getting error messages saying you can't access a non-static field in static main, so I changed them to static. I didn't know any other way to fix it. I know I should be utilizing methods more.

[Campbell Ritchie]

It is a serious mistake to make things static in order for them to compile. The correct response is to move things out of the main method. Have a look at this FAQ. You should have a Yahtzee class, Die class, Player class etc. Tthe main method can be even shorter than the FAQ suggests

public classYahtzeeDemo
{
   /*
    * empty to prevent instantiation
    * See http://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#d5e12106
    */
   private YahtzeeDemo
   {
      //empty
   }

   public static void main(String[] args)
   {
      new Yahtzee().startGame();
   }
}

You may pass information to the main method or the constructor or startGame method as described in the Java Tutorials.

[Jon Davis]

Thank you very much for your feedback, I will check them out.

[Campbell Ritchie]

You're welcome

[Jon Davis]

So if I am understanding correctly it should look a little more like what's below, I am really sorry I am having a hard time, and I really appreciate you all being patient with me and helping me out. Here is the code I have so far, I haven't transferred everything yet, but I am wanting to make sure I am closer to being on the right track.

public class YahtzeeTest
{
	public static void main( String args[] )
	{
		Yahtzee.startGame();
	}
}
class Yahtzee
{
	public static void startGame()
	{
		Die dice = new Die();
		dice.showDice( dice.rollDice() );
		System.out.println();
	}
}

import java.util.Random;
public class Die
{
	private Random rd = new Random();

	public int[] rollDice()
	{
		int[] dice = new int[5];
		for ( int a = 0; a < dice.length; a++)
		{
			dice[a] = 1 + rd.nextInt( 6 );
		}
		return dice;
	}
	public void showDice( int[] dice )
	{
		for ( int b = 0; b < dice.length; b++ )
		{
			System.out.print( dice[b] + " " );
		}
	}
}

[Campbell Ritchie]

A lot better, but startGame should not be static. Not Yahtzee.start... but new Yahtzee().start... Then you can if you wish run several games simultaneously.

Does anybody know whether the Random instance should be static? They used to say only use one instance per application otherwise you get repetitions of the same sequence. Is that still the case? Last time I asked, I was told no.

[Campbell Ritchie]

Add a println instruction at the end of the method which displays the five dice.

[Jon Davis]

This is what I have so far. I think my next move is going to be to make a method that handles the 3 rolls per turn, that way I don't have to write the same bit of code 13 times. I also think for now I am going to skip scoring and leave it as single player until I get the rest of the code right.
This is my Die class

import java.util.Random;
public class Die
{
	private Random rd = new Random();
	private int[] dice = new int[5];
	public int turn;

	//method to roll a single die
	public int rollDie()
	{
		int dieValue = 1 + rd.nextInt(6);
		return dieValue;
	}

	//method to create an array containing the 5 rolled dice
	public int[] firstRoll(){
		for (int a=0; a<dice.length; a++){
			dice[a]= rollDie();
		}
		return dice;
	}

	//method to hold the 5 dice
	public int[] diceArray(){
		return dice;
	}

	//method to display the array containing the dice
	public void showRoll( int[] dice ){
		for (int a=0; a<dice.length; a++){
			System.out.print(dice[a] + " ");
		}
		System.out.println();
	}
}

Yahtzee Class

import java.util.Scanner;

public class Yahtzee
{
	private Scanner input = new Scanner( System.in );
	public int s1, s2, s3, s4, s5;
	private int turn = 1;
	Die dice1 = new Die();

	//method to start a new game
	public void startGame(){
		System.out.println("Select which dice to re-roll.");
		dice1.showRoll( dice1.firstRoll() );
		while ( turn < 3 ){
			playerTurnInput();
			dice1.showRoll( dice1.diceArray() );
			}
	}

	//method to take players input for which dice to re-roll
	public void playerTurnInput(){
		s1 = input.nextInt();
		s2 = input.nextInt();
		s3 = input.nextInt();
		s4 = input.nextInt();
		s5 = input.nextInt();
		playerTurnReroll( dice1.diceArray() );
	}

	//method to determine which dice to re-roll
	public int[] playerTurnReroll( int[] dice ){
		if ( s1 == 1 ){
			dice[0] = dice1.rollDie(); //re-roll Die 1
		}
		if ( s2 == 1 ){
			dice[1] = dice1.rollDie(); //re-roll Die 2
		}
		if ( s3 == 1 ){
			dice[2] = dice1.rollDie(); //re-roll Die 3
		}
		if ( s4 == 1 ){
			dice[3] = dice1.rollDie(); //re-roll Die 4
		}
		if ( s5 == 1 ){
			dice[4] = dice1.rollDie(); //re-roll Die 5
		}
		turn++;
		return dice;
	}
}

The Class to run everything

public class YahtzeeTest
{
	public static void main( String args[] )
	{
		new Yahtzee().startGame();
	}
}

[Winston Gutkowski]

Jon Davis wrote:This is what I have so far. I think my next move is going to be to make a method that handles the 3 rolls per turn, that way I don't have to write the same bit of code 13 times.

Right. But make sure you keep clear (and separate) what the business of "being a die" and "being a Yahtzee game" are.

It might also be worth mentioning that your class is called Die, which is singular, but it contains several Dice, which is plural.

I also think for now I am going to skip scoring and leave it as single player until I get the rest of the code right.

NOW you're getting the idea: Get one thing working, test it, make adjustments if needed, and then (and ONLY then), move on.

Programming is an incremental process.

Winston

[Jon Davis]

Now I have it to where you get 3 rolls per turn, and 13 turns per game. I have this working the way I want it (for now any way), I think the next order of business will be the scoring portion, because I am tired of keeping score myself on a piece of paper. I think I am starting to get the hang of it, it is definitely way different than programming on a TI-83 (the only previous programming experience).

	//method to start a new game
	public void startGame(){
		turn = 1;
		while ( turn <= 13 ){
			newTurn();
		}
	}
	//method to start a new turn
	public void newTurn(){
		roll = 1;
		System.out.println("Select which dice to re-roll. " + turn );
		dice1.showRoll( dice1.firstRoll() );
		playerTurn();
	}

	//method to handle 3 rolls per turn
	public void playerTurn()
	{
		while ( roll < 3 ){
			playerTurnInput();
			dice1.showRoll( dice1.diceArray() );
		}
		turn++;
	}

[Jon Davis]

So this is what I have to check for small and large straights, and I was wondering if there was a better way to do this.

	public int sortForStraight( int[] diceArray )
	{
		int straightCount = 1;
		for ( int i = 1; i < diceArray.length; i++ )
		{
			if ( (diceArray[i-1]+1) == diceArray[i])
			{
				straightCount++;
			}
		}
		return straightCount;

	}

	public String[] scoreSmallStraight( int straightCount )
	{
		if( straightCount > 3 )
		{
			total = SMALL_STRAIGHT;
		}
		else
		{
			total = 0;
		}
		String stringtotal = String.valueOf(total);
		score[10] = stringtotal;
		return score;
	}

	public String[] scoreLargeStraight( int straightCount )
	{
		if( straightCount > 4 )
		{
			total = LARGE_STRAIGHT;
		}
		else
		{
			total = 0;
		}
		String stringtotal = String.valueOf(total);
		score[11] = stringtotal;
		return score;
	}