HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Dice simulation and counting pairs

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
countingsimulationdiceandpairs

Problem

This is my first time putting a code together for Java based on my own so i was hoping if you someone would be kind enough to review and provide feedback or constructive criticism on this code.

The goal is to roll 2 dice and 10k times. add pairs and display their frequency.

The code runs fine but maybe I am overlooking some logical error or a better way to do this.

```
/**
* Use the Random Number generator to write a java application that simulates a pair of dice.
* Throwing the pair of dice 10,000 times- Add the values for the pair
* Print the frequency at the end of 10,000 runs
* @author
*
*/

import java.util.Random;

public class diceSimulation {

public static void main(String[] args) {
/**
* Declare variables and store dice max roll in Thousand
*/
final int THOUSAND = 10000;
int counter, dice1, dice2;

//initiate an array with elements to keep count
int [] diceValue = new int [7];

// display welcome message. no other purpose but trial
welcome ();

// create new instance of random
Random rollDice = new Random();

/**
* Set counter to start from 1 and go till desired constant number
* Rolling two separate dices and storing values in dice1 & dice 2 respectively
*/
for (counter=1; counter<=THOUSAND;counter++){

dice1=rollDice.nextInt(6) + 1;
dice2=rollDice.nextInt(6) + 1;

// If statement to check if values are the same or not
if (dice1==dice2){
// IF values are same then go into for loop and store value in array element
for (int i=1; i<=6; i++){
if (dice1 == i && dice2 == i){
// add value for the number displayed into the array
diceValue [i] += 1;
}
}
}

}
// Display results totals of paired rolls
f

Solution

Minor things

The java naming standard for classes is CamelCase (DiceSimulation instead of diceSimulation)

A constant of THOUSAND is not helpful, especially when the value is 10,000. A more meaningful name, say ROLLCOUNT, would be better.

Similarly, diceValue is not clear. It contains the counts for each double occurence, so perhaps, doubleCounts - note the plural for an array indicating that it contains multiple values.

We have many dice but each one is a die -> die1 and die2.

Comments that just restate the obvious code can be omitted

e.g.

/**
* Declare variables and store dice max roll in Thousand
*/

// create new instance of random 

// If statement to check if values are the same or not


Coding

We are working with 6 sided dice so we only need 6 entries in the array to store the double counts not 7 - it is more standard to adjust the index when outputting (e.g., add 1) than to make the arrays larger and use then as 1 based arrays..

We can easily store the number of sides in a variable (constant) to make consistency easier - a named value instead of a literal 6 in multiple places - and even allow us to use this for dice with other numbers of sides if we desired.

We can put the 'roll' functionality into a function so that to reduce duplication and allow us to change the implementation if desired (say for testing)

We check if dice1 == dice2 but then in the loop check if both dice1 == i && dice2 == i - if dice1 == dice2 then the second check can never fail.

If we index into the array, we don't need the loop

if (die1==die2){
    doubleCounts[die1-1]++;
}


If have sized the array to the number of elements then we should loop from 0 not 1 (and single character variable names, even for loops, are generally frowned upon)

System.out.format is a nicer way - simpler to use than the concatenation - for writing the output line

import java.util.*;

public class DiceSimulation {

    private static Random rollDice = new Random();

    public static void main(String[] args) {

        final int ROLLCOUNT = 10000;
        final int SIDES = 6;
        int counter, die1, die2;

        //an array with elements to keep count
        int [] doubleCounts = new int [SIDES];

        // display welcome message. no other purpose but trial
        welcome ();

        /**
         * Set counter to start from 1 and go till desired constant number
         * Rolling two separate dices and storing values in dice1 & dice 2       respectively
         */
        for (counter=1; counter<=ROLLCOUNT;counter++){

            die1=roll(SIDES);
            die2=roll(SIDES);

            if (die1==die2){
                doubleCounts[die1-1]++;
            }

        }

        // Display results totals of paired rolls 
        for (int idx=0; idx<doubleCounts.length; idx++){
            System.out.format(" You rolled set of %d %d times\n",(idx+1), doubleCounts[idx]);
        }
    }
    private static int roll(int sides){
        return rollDice.nextInt(sides) +  1;
    }
    public static void welcome () {
        System.out.println("welcome to dice world!");
    }
}

Code Snippets

/**
* Declare variables and store dice max roll in Thousand
*/

// create new instance of random 

// If statement to check if values are the same or not
if (die1==die2){
    doubleCounts[die1-1]++;
}
import java.util.*;

public class DiceSimulation {

    private static Random rollDice = new Random();

    public static void main(String[] args) {

        final int ROLLCOUNT = 10000;
        final int SIDES = 6;
        int counter, die1, die2;

        //an array with elements to keep count
        int [] doubleCounts = new int [SIDES];

        // display welcome message. no other purpose but trial
        welcome ();

        /**
         * Set counter to start from 1 and go till desired constant number
         * Rolling two separate dices and storing values in dice1 & dice 2       respectively
         */
        for (counter=1; counter<=ROLLCOUNT;counter++){

            die1=roll(SIDES);
            die2=roll(SIDES);

            if (die1==die2){
                doubleCounts[die1-1]++;
            }

        }

        // Display results totals of paired rolls 
        for (int idx=0; idx<doubleCounts.length; idx++){
            System.out.format(" You rolled set of %d %d times\n",(idx+1), doubleCounts[idx]);
        }
    }
    private static int roll(int sides){
        return rollDice.nextInt(sides) +  1;
    }
    public static void welcome () {
        System.out.println("welcome to dice world!");
    }
}

Context

StackExchange Code Review Q#132710, answer score: 7

Revisions (0)

No revisions yet.