Your main()
function desperately needs to be busted up, for multiple reasons:
- You use a lot of variables, all of them declared at the top of
main()
. A human mind is only good at keeping track of about 7 things at a time, so this code is hard to follow.
- It violates the Single Responsibility Principle, by parsing the command line, throwing the dice, keeping the statistics, and printing the report. That's a lot of work for one function!
- Object-oriented code is preferred in Java. A
static
function avoids object-oriented thinking.
- It's unclear what you're simulating: what constitutes a trial, and what you mean by biggest/smallest sum.
A quick win in readability can be obtained by defining a Die
class: die.toss()
reads like English, whereas 1 + (int) ((Math.random() * (6 - 1)) + 1)
doesn't.
class Die {
// Since "throw" is a Java keyword, we use "toss"
int toss() {
return 1 + (int)(6 * Math.random());
}
}
A good start to taming the variables would be to declare each of them in the tightest scope possible. For example, theSum
, and diceSum
are only relevant inside the loop. count
is a loop counter, and it's much easier to recognize if you rewrite the loop as a for-loop. dice1
and dice2
are only relevant inside the inner loop, which is better expressed as a do-while loop.
for (int count = 0; count < Integer.parseInt(args[0]); count++) {
int theSum = 0, diceSum;
do {
int dice1 = 1 + (int) ((Math.random() * (6 - 1)) + 1);
int dice2 = 1 + (int) ((Math.random() * (6 - 1)) + 1);
diceSum = dice1 + dice2;
if (diceSum != 7) {
theSum = theSum + diceSum;
}
//System.out.println("the sum is "+theSum);
} while (diceSum != 7);
…
}
Now a picture is beginning to emerge. The inner loop is what you would call a simulation trial. Based on that, I would define a DiceSimulation
class with a runTrial()
method. Everything else just falls into place around that core function. ☺
public class DiceSimulation {
private int trials = 0, // Formerly count
min = Integer.MAX_VALUE, // Formerly lowest
max = 0, // Formerly finalSum
sum = 0; // Formerly totalSum
private Die die1 = new Die(),
die2 = new Die();
/**
* One trial consists of tossing a pair of dice until a sum of 7 is obtained.
* The result of the trial is the sum of all tosses up to, but not including,
* the toss that resulted in 7.
*/
public int runTrial() {
int trialSum = 0, pairSum; // Formerly theSum and diceSum
while (7 != (pairSum = die1.toss() + die2.toss())) {
trialSum += pairSum;
}
if (trialSum > max) {
max = trialSum;
}
if (trialSum < min) {
min = trialSum;
}
sum += trialSum;
trials++;
return trialSum;
}
public void report() {
System.out.println("After " + trials + " simulations: ");
System.out.println("Biggest sum: " + max);
System.out.println("Smallest sum: " + min);
System.out.println("The average is: " + (double)sum / trials);
}
public static void main(String[] args) {
int trials = Integer.parseInt(args[0]);
DiceSimulation sim = new DiceSimulation();
for (int count = 0; count < trials; count++) {
sim.runTrial();
}
sim.report();
}
}
(6 - 1)) + 1) == 6
. To throw a dice, you want to usenew Random().nextInt(6) + 1
– njzk2 Feb 5 '14 at 16:37