1
\$\begingroup\$

I have made a program to take x number of die and rolls them y number of times, then stores the data into an array so that I may output a CSV file. Everything works as intended, but I am having trouble figuring out how to increase the number of die to anything substantial. Right now I am using a switch, but linearly adding code like this seems inefficient, not to mention it will crash with amounts larger than 4 die. Is there some shortcut for adding variable number of switch statements? Any other methods would work as well, I am just not clever enough to come up with any as of yet.

import javax.swing.JOptionPane;

public class histogram {
public static void main(String[] M83cluster) {

    // # of die
    String N = JOptionPane.showInputDialog("How many dice would you like to roll?");
    int numofDie = Integer.parseInt(N);
    // # of rolls
    String M = JOptionPane.showInputDialog("how many times would you like to roll?");
    int numofRolls = Integer.parseInt(M);
    int maxValue = numofDie*6;
    int[] taco = new int[maxValue]; // for every die there will be at most 6 values.

    // rolls the die and obtains a value.
    for (int i=0;i<numofRolls; i++) {
        int oneTotalRoll = 0;
        for (int k=0;k<numofDie; k++) {
             oneTotalRoll += (int)(1+6*Math.random());
        }

        //int oneTotalRoll = (int) (valueofDice * numofDie);                
        System.out.println("ROLL: " + oneTotalRoll);            

        // for each roll, increment taco[] array.
        switch (oneTotalRoll) {
            case 4: taco[0] += 1;
                break;
            case 5: taco[1] += 1;
                break;
            case 6: taco[2] += 1;
                break;
            case 7: taco[3] += 1;
                break;
            case 8: taco[4] += 1;
                break;
            case 9: taco[5] += 1;
                break;
            case 10: taco[6] += 1;
                break;
            case 11: taco[7] += 1;
                break;
            case 12: taco[8] += 1;
                break;
            case 13: taco[9] += 1;
                break;
            case 14: taco[10] += 1;
                break;
            case 15: taco[11] += 1;
                break;
            case 16: taco[12] += 1;
                break;
            case 17: taco[13] += 1;
                break;
            case 18: taco[14] += 1;
                break;
            case 19: taco[15] += 1;
                break; 
            case 20: taco[16] += 1;
                break;
            case 21: taco[17] += 1;
                break;
            case 22: taco[18] += 1;
                break;
            case 23: taco[19] += 1;
                break;
            case 24: taco[20] += 1;
                break;
            case 25: taco[21] += 1;
                break;
        }
    }
    System.out.println("-------");

    String gorgon = null;  // prints outcome
    for (int g=0; g<maxValue ; g++) {
        String gigabolt = (taco[g] + ",");
        gorgon += gigabolt;
        // System.out.print(gigabolt);
    }
    if (gorgon.endsWith(",")) 
        gorgon = gorgon.substring(4, gorgon.length() - 1);
    System.out.print(gorgon);
}
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Simon Forsberg Jan 29 '16 at 22:05
6
\$\begingroup\$

Indentation

Your code's indentation is wrong, the class and method declaration should not be on the same line.


Whitespace

Your code has incorrect whitespace in places, for example:

int maxValue = numofDie*6;

and here:

case 4: taco[0] += 1;
    break;

The first one is missing whitespace between the operators, and the second one should have the taco[0] statement on a new line.


switch:

Your entire switch statement could be condensed to this:

taco[oneTotalRoll - 4] += 1;

All of your cases were testing if oneTotalRoll's value was 4 more than the array position, so in that case, you could just minus four from oneTotalRoll and use that as an index position.


Naming

You have variables named like taco , gorgon, N and M.

These should be named to things that are closer to their usage and what they are, for example:

  • taco into rollCount
  • gorgon into condensedList
  • N into stringDieCount
  • M into stringRollCount

String concatenation

Instead of string concatenating everything at the end, consider using an Array join to "glue" everything together with a common join (,).

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ That switch statement was mainly what I was after. It doesn't have to be minus four, I just happen to have that in there as an artifact from earlier versions trying to rid myself of the "null" being concatenated at the end. You made some very nice suggestions. \$\endgroup\$ – thekonductor Jan 22 '16 at 16:02
6
\$\begingroup\$

Hint

taco[oneTotalRoll - 4] += 1

Why start from 4 anyways? You say 'it will crash with amounts larger than 4 die', and that's due to ArrayIndexOutOfBoundsException, but what if the user only wanted to roll one dice, and that roll has a value smaller than four?

Parsing input

String N = JOptionPane.showInputDialog("How many dice would you like to roll?");
int numofDie = Integer.parseInt(N);

You should validate the input here, as the user can enter anything other than a number that Integer.parseInt(String) can handle. For example, you should re-prompt the user if the input contains entirely of spaces, or if a negative number is entered.

String concatenation

String gorgon = null;
for (int g=0; g<maxValue ; g++) {
    String gigabolt = (taco[g] + ",");
    gorgon += gigabolt;
}

This will also prefix a "null" to gorgon when it attempts to concatenate the first value:

gorgon = null + taco[0] + ","; // e.g. "null1,"

To sidestep that and the trailing ",", you can use a StringBuilder, or if you're on Java 8, String.join(CharSequence, CharSequence...) assuming you have converted your int[] results array into String[] values.

Java 8 and random value generation

Generating a stream of random integers in Java 8 is decidedly easy:

Random random = new Random();
int sum = random.ints(numofDie, 1, 7).sum();

Random.ints(long, int, int) lets us easily specify an IntStream of random integers with a given size and values range ([1, 7)), and the summation is done by calling IntStream.sum().

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Nice input. I will follow your suggestions. \$\endgroup\$ – thekonductor Jan 23 '16 at 0:26

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.