current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite
1

I've written a basic dice program in Java which takes a number of faces then randomly selects a number within that boundary. Here is the code:

package com.georgegibson.diceroller;

import java.util.Random;
import java.util.Scanner;

public class DiceRoller
{
    public static void main(String []args)
    {
        Random dice = new Random();
        Scanner input = new Scanner(System.in);
        int faces;
        int result;

        System.out.println("Dice Roller\n");
        System.out.println("How many faces does the dice have?");
        faces = input.nextInt();
        result = dice.nextInt(faces) + 1;
        System.out.println("\nThe dice rolled a " + result + ".");
    }
}

Seen as I am pretty new to Java (I come from C/C++ land), I should think this can be improved, but how?

EDIT: I know I should have a different package name, but I am going to get that domain (georgegibson.com).

share|improve this question

2 Answers 2

up vote 5 down vote accepted

In your code you have declared a variable dice which is a Random. This is an indication that you have a mismatch between the object and the implementation. The fact that you call it 'dice' implies you want something that is a Dice.... not a Random. A Dice should be able to "encapsulate" itself.

I would suggest a clean Java object oriented design would have a Dice class, something like:

public class Dice {
    private final Random rand;
    private final int faces;

    public Dice(int faces) {
        this.rand = new Random();
        this.faces = faces;
    }

    public int roll() {
        return 1 + rand.nextInt(faces);
    }

}

Now you have a class that represents a dice, and you can roll that dice, and you now have a main method that looks like:

public static void main(String []args)
{
    Scanner input = new Scanner(System.in);


    System.out.println("Dice Roller\n");
    System.out.println("How many faces does the dice have?");
    Dice dice = new Dice(input.nextInt());
    System.out.println("\nThe dice rolled a " + dice.roll() + ".");
}

You can reuse that dice, or create other dice with a different number of faces.

Note, there's always going to be a discussion about the singular is "Die", and the plural is "Dice". One Die, many Dice.... but I say "who cares" call it a Dice.... Creating a class called "Die" is just wrong ;-)

share|improve this answer
up vote 4 down vote

Unlike in C, in Java people usually declare the variables when they actuall need them; for example:

System.out.println("How many...");
int faces = input.nextInt();
int result = dice.nextInt(faces) + 1;

Also, what comes to braces, more idiomatic style is 

public class DiceRoller {
    ...

    public static void main(String[] args) {
        ...
    }
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.