DiceGame Code - which is better?

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]

Anything please??

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.

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.