DiceGame Code - which is better?

[Stephen Foy]

They are quite alike, but i believe one is better writte.
Which should i use, and can i improve on it?

code 1

[size = 1]import java.util.*;
 
class DiceGame {
 
int[] DiceNumbers = new int[3];
int[] HighestValues = new int[3];
public static DiceGame roll = new DiceGame();
public static String input;
 
public void score() {
int sum = 0;
for (int i = 0;i < HighestValues.length;i++) {
sum = ((sum + HighestValues[i]));
}
System.out.println("You score is: "+sum);
}
 
public void generateDice(int number) {
for (int i = 0;i < DiceNumbers.length;i++) {
number = (int) Math.floor(Math.random() * 6) + 1;
DiceNumbers[i] = number;
}
}
 
public void rollDice() {
int largest = Integer.MIN_VALUE;
int length = DiceNumbers.length;
int counter = 1;
int turns = 0;
boolean check = false;
 
while (counter <= 3) {
int rollNum = 0;
roll.generateDice(0);
 
if (counter == 2) {
length = DiceNumbers.length - 1;
} else if (counter == 3) {
length = DiceNumbers.length - 2;
}
 
rollNum++;
System.out.print("Roll "+rollNum+": |");
 
for (int i = 0;i < length;i++) {
if (DiceNumbers[i] > largest) {
largest = DiceNumbers[i];
}
HighestValues[i] = largest;
System.out.print(" "+DiceNumbers[i]+" | ");
}
 
if (turns == 1) {
System.out.println(" - Your number is "+largest+" you have no more rolls");
System.out.print("\n");
counter++;
turns = 0;
check = true;
} else { check = false; }
 
while (!check) {
if (turns < 1) {
System.out.print(" - Keep "+largest+" (y) or roll again (r): ");
Scanner in = new Scanner(System.in);
input = in.nextLine();
System.out.print("\n");
}
if (input.length() == 1 && input.charAt(0) == 'y') {
counter++;
check = true;
} else if (input.length() == 1 && input.charAt(0) == 'r') {
turns++; largest = 0;
check = true;
} else { System.out.print("Error invalid input"); check = false; }
}
}
}
 

public static void main(String[] args) {
boolean game = false;
while (!game) {
roll.rollDice();
roll.score();
 
System.out.print("Would you like to play again? y/n: ");
Scanner in = new Scanner(System.in);
input = in.nextLine();
 
if (input.length() == 1 && input.charAt(0) == 'y') {
game = false;
} else if (input.length() == 1 && input.charAt(0) == 'n') {
game = true;
} else { System.out.print("Error invalid input"); game = true; }
}
}
}

code 2:

import java.util.*;
 
class diceGame {
 
int[] diceRound = new int[3]; // Holds the random numbers
int[] values = new int[3];  // Holds the highest values
 
//  highestValue
//  Takes the values array, and adds together to return the score of the game.
// @return: returns sum of highest values
 
public int highestValue() {
int sum = 0;
for (int i = 0;i < values.length;i++) {
sum = ((sum + values[i]));
}
return sum;
}
 
// diceNumber
// Generates a random number
//@return: returns the randomNumber
 
int diceNumber() {
int realNum = (int) Math.floor(Math.random() * 6) + 1;
return realNum;
}
 
//  rollDice
//This methid plays the game, it rolls the dice once with 3 random numbers, gives user option
//to keep the highest or roll again, maximum 2 trys, then does this all the way down to one dice
//Assigns highest values to array "values".
 

public void rollDice() {
 
diceGame generate = new diceGame();
 
int largest = Integer.MIN_VALUE;
boolean UserRollsAgain = false;
int length = diceRound.length;
int moveRolls = 1;
int counter = 0;
String answer = " ";
 

while (moveRolls <= 3) {// Allows user to play the game 3 times
 
UserRollsAgain = false;
 
if (moveRolls == 2) {
length = length - 1; largest = 0;// If second roll then, two dice must be used
}
else if (moveRolls == 3) {
length = length - 1; largest = 0; // If third roll then, one dice.
}
 
// Loops program while counter < 2 and user rolls again
 
while (!UserRollsAgain) { // Rolls again if user says yes
 
System.out.print("Roll "+moveRolls+":  | ");
 
for (int i = 0;i < length;i++) {// Rolls dice and determines highest value
diceRound[i] = generate.diceNumber();// and assigns it to the array values.
System.out.print(diceRound[i]+" | ");
 
if (diceRound[i] > largest) {
largest = diceRound[i];
}
values[i] = largest;
}
 
// If statements, determines whether uses can roll again or not.
 
boolean check = false;
 
while (!check) { // While check is not successfull
 
if (counter < 1) {
System.out.print(" - Keep "+largest+" or roll again? y or r: ");
Scanner in = new Scanner(System.in);
answer = in.nextLine();
System.out.println(" ");
}
 
if (counter == 1) {
System.out.println (" - Your Number is "+largest+" You Have No more rolls\n");
UserRollsAgain = true;
moveRolls++;
counter = 0;
check =true;
}
 
else if (answer.length() == 1 && answer.charAt(0)=='r'|| answer.charAt(0)=='R') {
counter++;
largest = 0;
UserRollsAgain = false;
check = true;
}
 
else if (answer.length() == 1 && answer.charAt(0)=='y' || answer.charAt(0)=='Y') {
System.out.println ("(Your Number is "+largest+")\n");
UserRollsAgain = true; check = true;
moveRolls++;
}
 
else if (answer.charAt(0) != 'y' || answer.charAt(0) != 'Y' || answer.charAt(0) != 'R' || answer.charAt(0) != 'r') {
System.out.println ("Error"); check = false;
}
} // while (!check)
 
} // while (!UserRollsAgain)
 
} // while (moveRolls)
 
} //rollDice
 
//  main method
//runs the game, and displays highest value.
//gives user option to play game again
 
public static void main(String[] args) {
diceGame game = new diceGame();
boolean loopGame = false;
 
while (!loopGame) {
 
System.out.println("\n        [ Dice Game - Stephen Foy ]\n");
 
game.rollDice();
 
System.out.println("             Your Score is: "+game.highestValue()+"\n");
System.out.println("          [    Game Finished    ]\n");
 
System.out.print("Would you like to play again y or n: ");
Scanner in = new Scanner(System.in);
String answer = in.nextLine();
 

if (answer.length() == 1 && answer.charAt(0)=='y' || answer.charAt(0)=='Y') {
loopGame = false;
}
else if (answer.length() == 1 && answer.charAt(0)=='n' || answer.charAt(0)=='N') {
loopGame = true;
}
 
else { System.out.println("Error, exiting program"); loopGame = true; }
} // while (!loopGame)
 
} // main
 
}

[/size]

[Stephen Foy]

Anything please??

[Junilu Lacar]

Stephen,

Honestly, your code still needs a bit of work to make it clearer and easier to follow. As I mentioned in another thread, you should write code in such a way that it reads the way it is intended to work. Each method should have a single responsibility and consistent level of abstraction and detail. Assign methods names that reflect what they actually do. Your rollDice() method does not actually just roll the dice; it does almost everything.

My first-cut implementation of rollDice() would probably be something like:

public void rollDice() {
    for (int i = 0; i < diceCount; i++) {
        rollValues[i] = roll();
    }
}
 
private int roll() {
    return ... // a random number from 1..sides
}

But then again, this code is very procedural. If I really wanted OO code, I might try something like:

class Dice {
   private int sides;
   public Dice(int numSides) {
      sides = numSides;
   }
   public int roll() {
      return ... // a random number from 1 to sides
   }
}
 
public class DiceGame {
   public void play(int diceCount) {
       createDice(diceCount);
       while (!gameOver()) {
           rollDice();
           checkRolls();   
       }
       showResults();
   }
 
   public static void main(String[] args) {
       DiceGame game = new DiceGame();
       game.play(diceCountFromCommandLine(args));
   }
  ...
}

Notice that these first methods do not have much detail but they give you a high-level view of what happens in the program. If you need more detail, you would drill down to other methods. Those methods would have a lower level of abstraction and more detail. The trick is getting the right balance of detail and abstraction.

[Stephen Foy]

Thanks im gonna give it another bash, i think my problem is we havnt started OO programming yet, the ways i ave used are the ways we have been shown so far. Only been in java for 2-3 months. My problem is, linking methods, like you have there methods called in other methods.

How?

Thanks for the advice.