This is a working program I made to roll dice a specified number of times, show how many times each number (2-12) occurred, and the percentage of the total rolls that each number got.
I'm trying to make improvements to the efficiency of my code. I've already found one, as noted beside int[] results
on line 15. Are there any tools I could have used to make this code more efficient in terms of performance, or that could have saved me time? (Such as ArrayList, case/switch etc)
import java.util.Random;
import java.util.Scanner;
import java.text.DecimalFormat;
public class Dice {
public static void main(String[] args){
DecimalFormat df = new DecimalFormat("#.#"); //how to display percentage of results
Scanner sc = new Scanner(System.in);
System.out.println("How many times would you like to roll the dice?");
int rolls = sc.nextInt();
int[] counts = new int[rolls]; //creates an array of user-specified length (rolls)
sc.close();
for(int i = 0; i < counts.length; i++){ //populates said array with each roll of the dice
counts[i] = diceRoll();
}
int[] results = counters(counts); /*creates a new array with the returned array that counters(counts) gave us.
Seems redundant, but it is noticeably faster at high numbers to iterate through this array
than it is to access the counters method 3 times in the loop below (by using counters(counts) instead of results).*/
System.out.println("Roll\t\tCount\t\tFrequency"); //Headers for the results table
for(int i = 0; i < results.length; i++){ //iterates through our results array, prints how many times each number was rolled, along with its percentage of the total rolls
System.out.println((i+2)/*to start the table at 2, since rolling a 0 is not possible*/+"\t\t" + (results[i]) + "\t\t"+df.format(100.0*(results[i])/counts.length)+ "%");
}
}
public static int diceRoll(){ //rolls 2 6-sided dice and returns their sum
Random rand = new Random();
int dice1 = rand.nextInt(6)+1;
int dice2 = rand.nextInt(6)+1;
int roll = dice1+dice2;
return roll;
}
public static int[] counters(int[] arr){ //keeps track of how many times each possible number is rolled, puts the results into an array and returns the array
int c2=0;
int c3=0;
int c4=0;
int c5=0;
int c6=0;
int c7=0;
int c8=0;
int c9=0;
int c10=0;
int c11=0;
int c12=0;
for(int i = 0; i < arr.length; i++){
if(arr[i]==2)
c2++;
else if(arr[i]==3)
c3++;
else if(arr[i]==4)
c4++;
else if(arr[i]==5)
c5++;
else if(arr[i]==6)
c6++;
else if(arr[i]==7)
c7++;
else if(arr[i]==8)
c8++;
else if(arr[i]==9)
c9++;
else if(arr[i]==10)
c10++;
else if(arr[i]==11)
c11++;
else if(arr[i]==12)
c12++;
}
int[] rollCounts = new int[] {c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12};
return rollCounts;
}
}
-
\$\begingroup\$ Welcome to Code Review! Nice job on your first question. \$\endgroup\$Phrancis– Phrancis2016年03月18日 04:14:49 +00:00Commented Mar 18, 2016 at 4:14
-
1\$\begingroup\$ If you ever have multiple local variables that you are treating the same way, ask yourself: "Would an array, or hash, be appropriate? Would it let me replace many nearly identical lines with a single for loop?" The answer here is, "yes". \$\endgroup\$Phrogz– Phrogz2016年03月18日 05:09:34 +00:00Commented Mar 18, 2016 at 5:09
1 Answer 1
It is nice that you have separate methods for throwing the dice and computing the counts. However, diceRoll
can be simplified and counters
supersimplified; like this
public static int diceRoll() {
Random rand = new Random();
return rand.nextInt(6) + rand.nextInt(6) + 2;
}
public static int[] counters(int[] arr) {
int[] counterArray = new int[11];
for (int dice : arr) {
counterArray[dice - 2]++;
}
return counterArray;
}
(Edit Also, the name of each method should (preferably) begin with a verb: counters
\$\rightarrow\$ countResults
and diceRoll
\$\rightarrow\$ rollDice
.)
Hope that helps.