Should I create a single object for a common method call or use a throw away approach?

Question

I'm creating a little rpg fight program just for fun and to practice my java (I'm pretty new hehe).

I have a class called Dice with a method called d20 that rolls a random number between 1 and 20. The method will be used frequently by all other classes in the program.

My question is, should I create a single instance of the Dice class in my main method that can be called by all the other classes too. Does that work? Does it need to be declared as a public reference variable to work? Is this just a poor practice?

Should I be making the method static instead so that I don't need to make an instance of the object at all?

Or should I create the object when the call is to be made and then throw it away afterwards using garbage collection?

I'm not sure the first way even works. The latter two I don't know which is good practice.

My main motivation here is to understand what the best practice is and why.

Answer 1

Have any class that requires a random number source accept a Function<Integer, Integer> (or any equivalent functional interface you like) in its constructor.

Create a static method int rollDie(int sides) that generates a random number from 0 to sides, inclusive. This avoids the redundancy of rollD20, rollD8, etc. Interally this'll probably just be a call to Random.nextInt(sides + 1).

You can pass the static method to any class that needs it using a method reference:

new Character(Dice::rollDie)

For unit testing, you can easily replace rollDie with a predictable function:

new Character(sides -> 5);

This is equivalent to valenterry's approach but with less boilerplate and possibly more efficient, if the compiler/virtual machine can make use of the fact that the lambda refers to a static method. It's also a bit easier to reuse, since rollDie works with any interface containing one method that takes an Integer and returns an Integer, so you can use it in other contexts as well.

Answer 2

When the method doesnt remember a state a static Mehtod should be okay. java's shorthand random number method in the Math class (Math.random() ) is also a static method.

Answer 3

First, I would not call the class Dice. Rather call it DiceShaker or so and give it a method so you can call diceShaker.roll1W20() or even better, create different diceShakers (by composition/extending) for different purposes and make DiceShaker an interface.

Next, yes, it is good if you create a single instance that you pass to all the classes that need a DiceShaker. That way, you can easily test, if the DiceShaker works correct in unit tests. You can also give your classes special DiceShakers that can e.g. be more performant for edge cases.

E.g.:

public class Fighter {
    private DiceShaker diceShaker
    public Fighter(DiceShaker diceShaker) { this.diceShaker = diceShaker }
    public tryAttack(Fighter target) { if(diceShaker.roll1W20() > 10) this.attack(target) }
}

(Note that the logic for attacking or not attacking should not be in the Fighter class as above. This is just to simplify this example.)

Don't make this static. It will then be hard to unit test your diceShaker or provide different functionality for different classes later on or at runtime. Same drawbacks are there if you create the DiceShaker the moment you need it and discard it after. This is the opposite of Dependency Injection.

Even though many java libs use static functions, most of the time that is not good practice or just a workaround for other flaws in the java language (like apache.commons.io.FileUtils)

Answer 4

I would say you should initialize just one d20 object for your application.

The inspiration for this style is that a real life DM only has one D20. He doesn't have a whole box where he pulls one out, rolls it, and throws it away.

You should not assign a dice property to your RPG characters simply because RPG characters don't roll dice, the dungeonmaster does.

Therefore a static method like GetD20Roll():Number is the most clear. It does not suggest the monster itself is rolling dice.

Answer 5

I did following code, so I could measure the time in either option.

public static void multiple() {
    long start = System.currentTimeMillis();
    for (int i = 0; i < 100_000_000; i++) {
        Instance instance = new Instance();
        instance.rand();
    }
    long end = System.currentTimeMillis();
    long time = end - start;
    System.out.println("Multiple instance time: " + time);
}

public static void single() {
    long start = System.currentTimeMillis();
    Instance instance = new Instance();
    for (int i = 0; i < 100_000_000; i++) {
        instance.rand();
    }
    long end = System.currentTimeMillis();
    long time = end - start;
    System.out.println("Single instance time: " + time);
}

public static void statc() {
    long start = System.currentTimeMillis();
    for (int i = 0; i < 100_000_000; i++) {
        StaticInstance.rand();
    }
    long end = System.currentTimeMillis();
    long time = end - start;
    System.out.println("Static instance time: " + time);
}

After run 100,000,000 time. The result was:

Multiple instance time: 2879
Single instance time: 2714
Static instance time: 2770

As expected, multiple instances is the worst option. Its important that in my samples, the constructor was dumb and light. If you got a heavy constructor, it will be big a pain for performance to multiple instances.

Single instance was the best one. But you must have a reference to the instance.

Static was a little worst than single instance.

Best regards, Felipe