Dice thrower with methods

Question

I'm currently doing some exercises from my study book, and one of the tasks was to: Write a dice class "Dice" which has got a value between 1-6. There should also be a constructor that picks a random value, and a method roll() that also picks a random value. The method getValue() should also be created, which is going to be used to retrieve the value being shown. Write a test class for this program.

Edit* I moved the randomizer up to the constructor, leaving the roll method empty. What should I do in the roll() method when the constructor already randomizes?

This is my code so far:

public class Dice {

    int value;
    int currentRoll;

    public Dice() {
        Random rand = new Random();
        this.value = 1;
        this.currentRoll = rand.nextInt(6) + 1;
    }

    public int roll() {
        Random rand = new Random();
        this.currentRoll = rand.nextInt(6) + 1;
        return this.currentRoll;
    }

    public int getValue() {
        return this.currentRoll;
    }
}

What I do not understand is: why should you random the value both in the constructor, and in the roll() method? Also, what am I missing out on?

@MadProgrammer it's OK to have any default value (`null` or `0`) for `currentRoll` since the dice hasn't been rolled yet — Andrew Tobilko, Oct 26 '18 at 10:41
You could pass the `Random` as constructor parameter and store it as instance variable. — , Oct 26 '18 at 10:43
@AndrewTobilko Not really, a "die" doesn't have a "default" value and since it's a primitive, it can't be `null`. By randomise the value in the constructor you overcome the need to have "additional" state rules — MadProgrammer, Oct 26 '18 at 10:45
Remember, this is an exercise. The exercise is asking you to complete a given task in a specific way. In real life, what is the default value of a die anyway? By randomising the value in the constructor, you are reducing the need to have additional state rules or error conditions if the user calls `getValue` before `roll`. You may also find that the exercise later describes "why" it avoided calling `roll` from the constructor directly — MadProgrammer, Oct 26 '18 at 10:48
I've edited my post since I moved the randomizer to the constructor. — Griezy, Oct 26 '18 at 10:54
As a "suggestion" don't keep creating instances of `Random`, make it an instance field of the class, this way you should get a better randomisation — MadProgrammer, Oct 26 '18 at 11:00
In your `Dice` class, you don't need the `value` field, it is never used and always 1. — NickJ, Oct 26 '18 at 11:00
*"What should I do in the roll() method when the constructor already randomizes?"* - Randomise the value again, because the die has been rolled, you need a new value — MadProgrammer, Oct 26 '18 at 11:01
@NickJ I "suspect" `currentRoll` should be `value`, but in either case, they don't need both — MadProgrammer, Oct 26 '18 at 11:01

score 2 · Answer 1 · answered Oct 26 '18 at 10:47

2

Why pick a random value in the constructor? Well, frankly, because it's the requirement of the exercise.
Why do they have this requirement? If I'd had to guess, it's in order to simulate the fact that a die will always have some face up (i.e., have a value), whether you explicitly rolled it or not, but if you want a definitive answer, you'd have to ask the book's author what he or she had in mind.

You can achieve this by calling roll in the constructor. Also, note that you have redundancy with the value member that's initialized, but never used:

public class Dice {
    private int currentRoll;

    //Constructor
    public Dice() {
        roll();
    }

    // methods...

answered Oct 26 '18 at 10:47

Mureinik

297,002
52
306
350

I agree, right up to the point of calling `roll` in the constructor, but that's a nit-pick which is probably beyond the scope of the question ;) – MadProgrammer Oct 26 '18 at 10:49
I've edited my post so you could take a look at my current code. Value is never used I know, should I remove it completely? – Griezy Oct 26 '18 at 10:52
@Griezy Neither's roll. The point is, it's an exercise, you've been asked to carry it out in a specific manner, this many lead into additional exercises later. Don't try to optimise the solution to early, they might be leading you some where ;) – MadProgrammer Oct 26 '18 at 10:57
Note that it is **bad habit** to put calls to overridable methods in the constructor. [See here why](https://stackoverflow.com/questions/3404301/whats-wrong-with-overridable-method-calls-in-constructors). – MC Emperor Oct 26 '18 at 11:04

score 0 · Answer 2 · answered Oct 26 '18 at 11:16

The requirement is that both the constructor and the roll() method pick a random value. They do the same thing. So in order to avoid unnecessary code duplication, the constructor could, for instance, call the roll method.

Caution: it is bad habit to put calls to overridable methods in the constructor. This post explains why that is.

Your code could look like this:

class Dice {

    private int value;

    // You could create the Random instance here once, instead of everytime
    // recreating it
    private Random r = new Random();

    public Dice() {
        roll();
    }

    // You could also change the return type to and int and return this.value
    public final void roll() {
        this.value = this.r.nextInt(6) + 1;
    }

    public int getValue() {
        return this.value;
    }
}

I do not know if I completely agree with the requirements, but since they're requirements, you should concede to them.

Dice thrower with methods

2 Answers2