5
\$\begingroup\$

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;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Nice job on your first question. \$\endgroup\$ – Phrancis Mar 18 '16 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 Mar 18 '16 at 5:09
2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.