A type of game simulation dropping disks

Question

Quincunx is that little game where you drop a ball or disk down rows of nails in a pyramid formation until they land in a slot. This was a project for my class, but it's already been submitted. I am just looking for personal feedback since the professor does not give much in-depth feedback on these assignments. The goals of the program were to:

1. Ask the user for how many slots they want the game to have (which also determines how many rows of nails there are).

2. Ask how many balls/disks they wish to drop.

3. Create a path for each ball dropped to determine what slot it lands in.

4. Create a histogram to visually represent how many balls are in each slot. It should look like a bell curve.

 import java.util.Scanner;
public class Quincunx { 
public static void main(String[] args) {
 Scanner ballNumber = new Scanner(System.in); //Scanners for slots and number of balls
 Scanner slotNumber = new Scanner(System.in);
 System.out.println("Enter the Number of slots you want.");
 int numberOfSlots = slotNumber.nextInt();
 System.out.println("Enter the number of balls you wish to drop into the slots");
 int numberOfBalls = ballNumber.nextInt();
 System.out.println("You are dropping " + numberOfBalls + " balls into " + numberOfSlots + " slots"); //Displaying what the user entered
 int Slots[] = new int[numberOfSlots]; //create an array for the slots based on how many slots the user wants
 int rowsOfNails = numberOfSlots - 1; //create the rows of nails to determine path size
 eachBallPath(numberOfBalls, rowsOfNails, Slots);
 for (int i = 0; i < Slots.length; i++) {
 System.out.println("Slot " + i + " has " + Slots[i] + " balls in it");
 }
 makeAsteriskHistogram(Slots);
}
public static void eachBallPath(int numberOfBalls, int rowsOfNails, int Slots[]) {
 for (int i = 1; i <= numberOfBalls; i++) { //loop to make a path fr each ball
 String path = new String(pathTaken(rowsOfNails)); //generate and print the path of a ball
 System.out.println("Ball " + i + " took the path " + path + " and landed in slot " + ballInSlot(path, Slots));
 }
}
public static int ballInSlot(String path, int Slots[]) {
 //Take the path string and find the number of times it went right to decide which slot it landed in
 int frequencyOfRight = 0;
 char R = 'R';
 for (int i = 0; i < path.length(); i++) {
 char currentLetter = path.charAt(i); // compare each character of the string to 'R' and add to frequency if R is detected
 if (currentLetter == R)
 frequencyOfRight++;
 }
 Slots[frequencyOfRight] = Slots[frequencyOfRight] + 1; //the number f Rs is the slot, add a value of 1 to a slot each time a ball lands there
 return frequencyOfRight;
}
public static String pathTaken(int rowsOfNails) {
 String path = new String(""); //start the path a ball takes
 for (int i = 0; i < rowsOfNails; i++) { //do this for every row of nails, as each row is a chance to go left or right
 double random = Math.random(); //random number to determine left or right
 if (random < 0.5)
 path = path + "L";
 if (random >= 0.5)
 path = path + "R";
 }
 return path;
}
public static void makeAsteriskHistogram(int Slots[]) {
 //find the max value within the dataset to create a scale for the histogram
 int max = Slots[0];
 for (int i = 0; i < Slots.length; i++) {
 if (Slots[i] > max)
 max = Slots[i];
 } //end of the loop that found the max value for scaling the histogram
 for (int k = 0; k < Slots.length; k++) {
 System.out.print("_______"); //lines to clearly seperate the graph from the previous output
 }
 System.out.println();
 System.out.println("----------Number of Balls in Slot----------"); //space for aesthetic/organization 
 System.out.print("#######|"); //aesthetic
 for (int i = 1; i <= max; i++) { //create a horizontal scale for the values of the bars
 if (i < 10)
 System.out.print("0"+ i + " ");
 else
 System.out.print(i + " ");
 }
 System.out.println();
 System.out.print("#######|--");
 for (int i = 0; i <= max; i++) { //add dashed lines under the value scale for aesthetic
 System.out.print("-----");
 }
 System.out.println(); //new line for the labels on the other axis
 for (int i = 0; i < Slots.length; i++) {
 if (i < 10)
 System.out.print(" Slot " + i + "|");
 if ( i >= 10)
 System.out.print("Slot " + i + "|");//create the label
 for (int j = 1; j <= Slots[i]; j++) { //nested loop to print the appropiate amount of *'s to show how many in a slot
 System.out.print(" * ");
 }
 System.out.println();// new line for next slot
 }
} //end of makeAsterisksHistogram method
} //End of Class file

• user214085: If you wrote this without an IDE, please make sure to use one in the future. The linter will help you greatly with indenting and coding standards errors.

Answer 1

Don't instantiate a new Scanner object for every input

You only ever need 1 Scanner.

Format Strings

You can use System.out.printf to print formatted strings, such as:

System.out.printf("You are dropping %d balls into %d slots\n", numberOfBalls, numberOfSlots); 

Put comments above the line of code, not in the same line.

// like this
someCode();
someCode(); // NOT like this

Use java naming standards

private & method scoped variables should begin with a lower case letter. The formatting in stackexchange gives you a hint, it thinks Slots is a class since it begins with an uppercase:

// should be int slots[].
int Slots[] = new int[numberOfSlots];

Give names that make sense

eachBallPath doesn't really make sense as a method name. printEachBallPath would make more sense. printBallPaths would make even more sense.

Or if you'd like, calculateBallPaths, and have the method return an Array that can be printed elsewhere.

Same for ballInSlot and pathTaken. Maybe getBallInSlot and getPathTaken. If you don't like using get for methods other than getters, I'd suggest calculatePathTaken & calculateBallInSlot.

Add method documentation also known as JavaDocs when a method is unclear.

Taking the a glance at the method ballInSlot from the view of another programmer:

public static int ballInSlot(String path, int Slots[]) {
...
return frequencyOfRight;

It's really hard to tell what this method is actually doing. If you cannot make it clear through naming, it might be an indication the method is too complicated and should be split into multiple methods.

However sometimes this isn't possible / doesn't help. In these cases you should:

Add a method documentation AKA Java Docs:

/**
 * explanation of the method
 * @param path explanation of the parameter 'path'
 * @param Slots explanation of parameter 'Slots'
 * @return what does the method return?
 */

IMO every public method should be documented, and every unclear private method. Some people believe every method should be documented. However it's pretty standard to document methods that are unclear.

Regarding the code below:

always use curly braces even when the code is 1 length long.

A good explanation:

Skimping on braces might save you a few keystrokes the first time, but the next coder who comes along, adds something to your else clause without noticing the block is missing braces is going to be in for a lot of pain.

Write your code for other people.

use else if or else whenever it makes sense to do so

You can use shorthand +=, -=, *=, /= etc, instead of variable = + variable etc.

if (random < 0.5)
 path = path + "L";
if (random >= 0.5)
 path = path + "R";

• Daedric_Man_guy: thank you. I agree that my method names are probably very confusing, I just didnt think of what else to name them at the time. I wanted to avoid doing something like getBalInSlot as that is usually used for getter methods in the programming class this was for Also, my professor advised against brackets for single-line if statements and similar, why should they actually be used? Also thanks for pointing out I didnt use proper convention for the slots array, I tried to follow it elsewhere and I guess that slipped by me. What kind of documentation should be added to the methods?
• dustytrash: @Daedric_Man_guy I've updated my answer with more explanations
• Daedric_Man_guy: I would appreciate a little more explanation of what you mean by documentation, I have comments that explain what each method is trying to do. Is proper documentation meant to a few steps further and break down how each line of code performs the intended task and why the task is being performed the way it is, instead of just saying "loop to make a path for each ball", maybe instead make it say "This loop generates a random number between 0 and 1 for each row of nails on the board to simulate a random turn. If the number if less than 0.5, the ball goes left, if it is >=0.5, it goes right" ?
• Daedric_Man_guy: Also, is comments above the lines of code standard convention? I ask this because my textbook always puts comments next to the relevant line of code, and my professor does too since he teaches out of the textbook
• dustytrash: @Daedric_Man_guy I've never heard of comments being on the same line. It's pretty standard to have them on the line above. It's possible your instructor is very old fashioned, or maybe used languages other than Java. In any you should follow what your instructor does & make your own decisions after the course

Answer 2

Thanks for sharing your code.

Unfortunately your code has a lot of issues to be addressed:

Code Style

Code format

Your code looks messy because you don't use indentation to structure blocks. Learn how to invoke the auto formatter of your IDE or how to apply it automatically when saving.

Comments

You have lots of inline comments that repeat what the code does. Such comments are useless at best, dangerous in long living projects since the tend to stay unchanged while the code develops. This way they turn into lies because they now say something dirrerent then the code itself.

So write inline comments that explain why the code is like it is.

When ever you feel you need an inline comment think twice if better identifier names could support better readability.

Naming

Finding good names is the hardest part in programming. So always take your time to think about your identifier names.

Naming conventions

Please keep to the Java Naming Conventions. This dies nt only include the camelCase style, but also how identifiers are "constructed". E.g. names of methods should start with a verb and explain what their main task is.

example:
your method eachBallPath() should better be printPathsOfAllBalls() or
method ballInSlot() should better be findFrequencyOfRight().

Choose names from the problem domain

Always use expressive names for your identifiers taken from the problem domain, not from the technical solution. An example from you code is the variable currentLetter which would better be currentTurn.

Avoid Single letter names

Do not use single letter names or abbreviations. There are only few exceptions like loop variables or very common abbreviations. While reading this code your brain has to do additional work to resolve single letters to their meaning.

E.g. you have a valiable R that could be rightTurn.

In conjunction with the previous issue the better code would read as:

 int frequencyOfRight = 0;
 char rightTurn = 'R';
 for (int i = 0; i < path.length(); i++) {
 char currentTurn = path.charAt(i); 
 if (currentTurn == rightTurn)
 frequencyOfRight++;
 }

General Coding

avoid magic numbers

Your code uses lots of literal values. Usually this applies to numbers only but I for myself extend this to Strings too. You should create constants for them with expressive names to make your code more readable.

example:

private static final String RIGHT_TURN = "R";
private static final String LEFT_TURN = "L";
private static final double TURN_RIGHT_CHANCE = 0.5;
// ... 
 String path = new String(""); 
 for (int i = 0; i < rowsOfNails; i++) { 
 double random = Math.random(); 
 if (random < TURN_RIGHT_CHANCE)
 path = path + LEFT_TURN;
 if (random >= TURN_RIGHT_CHANCE)
 path = path + RIGHT_TURN;
 }
 return path;

avoid unnecessary evaluations

The code above has another issue: you have two if condition to separate the opposit cases. Why donn't simply use the else key word?

 if (random < TURN_RIGHT_CHANCE)
 path = path + LEFT_TURN;
 else
 path = path + RIGHT_TURN;

You might also go one step further an use the "elvis operator" (ternary operator):

 path = random < TURN_RIGHT_CHANCE ? LEFT_TURN : RIGHT_TURN;

avoid explicit String object creation

The code String path = new String("") is in your small program logically the same as String path = "". But it can have subtle impact on performance and logic. Just don't do it unless you really know why.

don't share singelton resources

In the beginning you create two instances of the Scanner class wrapping the same System.in object. This is a really bad idea.

On top these Scanner instances have misleading names. They pretend to hold the number of balls and the number of slots respectively, but in fact they hold the user input.

• Daedric_Man_guy: Thank you, so basically similar stuff the other guy pointed out, mostly style errors (due to my professor not covering standard style conventions in too much depth. ) Also, I dont think jGrasp has an autoformat, or at least I am not aware of it after digging through some menus (We are recommended to use jGrasp in this course) and then some potential issues with readability and variable names and the two scanner issue
• Imus: @Daedric_Man_guy If you're "recommended" in the meaning of "forced with kinder words" then use jGrasp. If it's really only a recomendation and you're free to choose your own, I suggest IntelliJ or Eclipse instead. They're a bit harder to install and setup the first time, but will help you a lot more with coding standards later on (with just a few keystrokes you can have those auto-format your entire classes to the more usual Java coding standards).
• Timothy Truckle: @Imus "They're a bit harder to install and setup the first time" --- Um... I don't know about IntelliJ but the install process of eclipse is unzipping the archive. How could anything be easier?