2
\$\begingroup\$

I wrote a simple dice game with the purpose to practice arrays and classes. I would like some critique on my code structure and quality - as well as thoughts on my thoughts to improve it.

Here's the code:

import java.util.Random;
import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.io.IOException; 
public class ceeLo
{
 public static void main(String[] args) throws IOException
 {
 //declaration block
 BufferedReader BR = new BufferedReader (new InputStreamReader(System.in));
 Player human = new Player();
 Player computer = new Player();
 double wallet = 100.0;
 double bet = 0.0;
 System.out.println("What's your name?: ");
 human.setName(BR.readLine());
 computer.setName("Computer");
 System.out.println("\nWelcome to Cee Lo " + human.getName() + "\n");
 //------------------game loop------------------//
 do
 {
 System.out.println("\nWallet: " + wallet);
 System.out.println("How much would you like to bet?\n");
 bet = input(wallet);
 human.playerRolls();
 computer.playerRolls();
 showRoll(human);
 showRoll(computer);
 int scoreHuman = gameLogic(human.getDieValues(), human);
 int scoreComputer = gameLogic(computer.getDieValues(), computer);
 String victor = calculateWinner(scoreHuman,scoreComputer, human, computer);
 System.out.println("\nThe winner is: " + victor);
 wallet = adjustWallet(victor, wallet, bet, human, computer);
 } while (wallet > 0.0);
 System.out.println("\nWallet: " + wallet);
 System.out.println("Goodbye!");
 }
 /**
 @param int[] array = sorted array of all die values from player class 
 @param Player x = player object
 gameLogic returns an int with the score a player has based on their roll
 */
 public static int gameLogic(int[] array, Player x)
 {
 int playerScore = 0;
 // 4, 5, 6 is an automatic win
 if(array[0] == 4 && array[1] == 5 && array[2] == 6)
 {
 System.out.println(x.getName() + " won!");
 playerScore = 100; //high number not achieved by any other rolls
 }
 // 1, 2, 3 is an automatic loss
 else if (array[0] == 1 && array[1] == 2 && array[2] == 3)
 {
 System.out.println(x.getName() + " loses!");
 playerScore = -100; //no roll can give neg value
 }
 //sorted array may either have two of same number at beginning or end - leftover is score
 else if (array[0] == array[1] && array[1] != array[2])
 {
 playerScore = array[2];
 System.out.println(x.getName() + " got: " + playerScore);
 }
 else if (array[1] == array[2] && array[2] != array[0])
 {
 playerScore = array[0];
 System.out.println(x.getName() + " got: " + playerScore);
 }
 //triple of same number worth more than above scores. Multiply score by 10 to signify this. 
 else if (array[0] == array[1] && array[1] == array[2])
 {
 System.out.println(x.getName() + " rolled trip " + array[0] + "'s!");
 playerScore = array[0] * 10;
 }
 // No score: roll, print roll, recursive call to gameLogic with new die values
 else
 {
 System.out.println(x.getName() + " got: nothing. Re-rolling...");
 x.playerRolls();
 showRoll(x);
 playerScore = gameLogic(x.getDieValues(), x);
 }
 return playerScore;
 }
 /**
 @param int a = human's score
 @param int b = computer's score
 @param Player x = human player
 @param Player ai = computer
 calculateWinner compares the scores attained by human and player and
returns string with winner's name.
 */
 public static String calculateWinner(int a, int b, Player x, Player ai)
 {
 String winner = " ";
 if(a > b)
 {
 winner = x.getName();
 }
 else if ( a < b)
 {
 winner = ai.getName();
 }
 else
 {
 System.out.println("Tie!");
 winner = "Nobody";
 }
 return winner;
 }
/**
@param String s = victor from main, which stores name of winner of round
@param double max = wallet amount (initialized to 100 to start)
@param double wager = bet amount
@param Player x = human player
@param Player ai = computer
adjustWallet removes lost bet or adds winnings to wallet amount
*/
 public static double adjustWallet(String s, double max, double wager, Player x, Player ai)
 {
 if (s.equals(x.getName()))
 {
 max += wager;
 }else if (s.equals(ai.getName()))
 {
 max -= wager;
 }
 return max;
}
/**
 @param Player x = player object instance
 showRoll prints out player's roll values.
*/
 public static void showRoll(Player x)
 {
 System.out.println(x.getName() + " rolls... ");
 for (int i = 0; i < 3; i++)
 {
 System.out.println(x.getDieValues()[i] + " ");
 } 
 System.out.println();
 }
/**
@param double max = wallet amount
input takes in the amount a player wants to bet and returns it if it's more than 0 and within wallet amount
*/
public static double input(double max) throws IOException
 {
 BufferedReader BR = new BufferedReader(new InputStreamReader(System.in));
 double value;
 value = Double.parseDouble(BR.readLine());;
 if(value < 0 || value > max)
 value = validation(value, max);
 return value;
 }
/**
@param double x = amount a player wants to bet
@param double max = wallet amount
validation tells player if they input an amount 0 or less, or more than they have in the wallet 
and takes in input until it meets the rules. Returns to input method. 
*/ 
public static double validation(double x, double max) throws IOException
 {
 BufferedReader BR = new BufferedReader(new InputStreamReader(System.in));
 while( x < 0 || x > max)
 {
 System.out.println("\nValue must be greater than 0! Max bet = whole wallet.\n");
 x = Double.parseDouble(BR.readLine());
 }
 return x;
 }
}
class Die 
{
 private int sides;
 private int value;
 Die(int numSides)
 {
 sides = numSides;
 roll();
 }
 void roll() 
 {
 Random rand = new Random();
 value = rand.nextInt(sides) + 1;
 }
 int getSides()
 {
 return sides;
 }
 int getValue()
 {
 return value;
 }
}
class Player
{
 private String name;
 private int[] dieValues = new int[3];
 void playerRolls()
 {
 final int numDie = 3;
 final int numSides = 6;
 Die[] dieArray = new Die[numDie];
 for (int i = 0; i < dieArray.length; i++)
 {
 dieArray[i] = new Die(numSides);
 }
 for (int i = 0; i < dieArray.length; i++)
 {
 dieValues[i] = dieArray[i].getValue();
 }
 selectionSort(dieValues);
 } 
 int[] getDieValues()
 {
 return dieValues;
 }
 void setName(String pname)
 {
 name = pname;
 }
 String getName()
 {
 return name;
 }
/**
@param int[] array = array to be sorted
selectionSort sorts an array's contents from lowest to highest
*/ 
 void selectionSort(int[] array)
 {
 int startScan, index, minIndex, minValue;
 for (startScan = 0; startScan < array.length-1; startScan++)
 {
 minIndex = startScan;
 minValue = array[startScan];
 for( index = startScan + 1; index < array.length; index++)
 {
 if(array[index] < minValue)
 {
 minValue = array[index];
 minIndex = index;
 }
 array[minIndex] = array[startScan];
 array[startScan] = minValue;
 }
 }
 }
}

I would switch from arrays to arraylist to use the built in sort method, change the game loop to include a menu option to quit mid-game, and have better input validation perhaps with an overloaded input method to not have the program crash. Also perhaps switch from buffered reader to scanner (I just used buffered reader to see how it works as I've always used scanner up to this point)

asked Jun 6, 2020 at 15:37
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

well, your code looks fine, but there are few things I would like to talk about.

  • The class names allways goes using UpperCamelCase, at least in Java it is very common.
  • Usually we have the parameter name equally named to the class attribute:
void setName(String pname) {
 name = pname;//you should not. plus your IDE may have getters/setters generator
}
//use instead
void setName(String name) {
 this.name = name;//better, according to what most Java programmers are used to see
 //however, it is no big deal. (but more work for you)
}
  • The brackets:
class ceeLo
{//C++ style
}
class CeeLo {
//Java style
}
  • Normally variable values are assigned in the constructor instead of directly
private int[] dieValues = new int[3];//no, at least if it is not constant
//yes
public Player() {
 this.dieValues = new int[3];
}
  • Finally, there:
void playerRolls()
 {
 //as the method is called (and is called often)
 //the program will create and destroy these constants. so, you should declare
 //them outside the method
 final int numDie = 3;
 final int numSides = 6;
//example
private static final int numDie = 3;
private static final int numSides = 6;
void playerRolls()
 {//... use those here

I hope it has been helpful.

answered Jun 6, 2020 at 17:08
\$\endgroup\$
2
\$\begingroup\$

Overall, a solid effort. You've already listed the major areas for improvement.

  1. Switch from arrays to Lists to use the built-in sort method.

  2. Change the game loop to include a menu option to quit mid-game.

  3. Validate the input.

Now for my comments.

The convention is that class names in Java start with a capital letter. The ceeLo class should be named CeeLo.

Variable names and method names in Java start with a lower case letter. The

BufferedReader BR

should be

BufferedReader br

or

BufferedReader reader

All of the methods in the CeeLo class are static. While main must remain static, you should use non-static methods and create an instance of the CeeLo class to call the methods.

Use blank lines sparingly to separate groups of code. Here's an example of what I mean from your main method.

 do {
 System.out.println("\nWallet: " + wallet);
 System.out.println("How much would you like to bet?\n");
 bet = input(wallet);
 human.playerRolls();
 computer.playerRolls();
 showRoll(human);
 showRoll(computer);
 int scoreHuman = gameLogic(human.getDieValues(), human);
 int scoreComputer = gameLogic(computer.getDieValues(), computer);
 String victor = calculateWinner(scoreHuman, scoreComputer, human, computer);
 System.out.println("\nThe winner is: " + victor);
 wallet = adjustWallet(victor, wallet, bet, human, computer);
 } while (wallet > 0.0);

In a Javadoc, the explanation comes first, followed by the @param descriptions, followed by the @return description.

You can use basic HTML in your explanation.

Here's a reworked example from your code.

/**
 * The input method takes in the amount a player wants to bet as a
 * <code>String</code> and returns it as a <code>double</code> if it's more than
 * zero and within the wallet amount.
 *
 * @param max = wallet amount
 * @return value
 */
answered Jun 6, 2020 at 17:10
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.