Comparing int's in java

Question

I have this script to simulate 2 die rolls in java. This is done twice, once by user and once by computer(both automatically). The program outputs the rolls and sums them up. I however cannot get an if/else statement to work to compare the rolls and determine the winner/ or tie. So far I have:

import java.util.Scanner;
import java.util.Random;
import java.text.DecimalFormat;
/*
Program to simulate die roll
*/
public class Dice
{
Scanner scan = new Scanner (System.in);
Random generator= new Random();
int roll1;
int roll2;
int roll3;
int roll4;
int addroll1;
int addroll2;

public void userdieroll() // Simulates users role
{
    roll1 = (generator.nextInt(7) +1); // Generate number from 1-6
    System.out.println("Your first roll is "+roll1+"");// Says users first role
    roll2 = (generator.nextInt(7) +1); // Generate number from 1-6
    System.out.println("Your second roll is "+roll2+"");// Says users second roll
    addroll1=  roll1 +roll2;// Sums users roles
    System.out.println("The sum of your two roles is "+addroll1+" \n");
}
public void compdieroll()// Simulates computers role
{
    roll3 = (generator.nextInt(7) +1); // Generate number from 1-6
    System.out.println("The computers first role is "+roll3+""); // Says computers first role
    roll4 = (generator.nextInt(7) +1); // Generate number from 1-6
    System.out.println("The computers second role is "+roll4+""); // Says computers second role
    addroll2=  roll3 +roll4;// Sums computers roles
    System.out.println("The sum of the computers roles is "+addroll2+""); 
}
public void findwinner()
{
        if (addroll1 == addroll2)
        {
            System.out.println("Its a tie!");
        }
        else
        {   
           if (addroll1 > addroll2)
           {
               System.out.println("You Won!");
           }

           else
           {
               System.out.println("You lost!");
           }

}
}
public static void main(String[] args)
{
    Dice userroll = new Dice();
    userroll.userdieroll();
    Dice comproll = new Dice();
    comproll.compdieroll();
    Dice looper = new Dice();
    looper.findwinner();

}

}

Answer 1

In your class there are both the user dice than the computer dice variables. in your main you should create only one class:

Dice d = new Dice();
d.userdieroll();
d.compdieroll();
d.findwinner();

your code creates three different instances of Dice with three different roll1, roll2, ... when you write

...
Dice looper = new Dice();
looper.findwinner();

it compares varlable looper.addroll1 with looper.addroll2 (i.e. the ones inside the looper class), but these variables are still 0 (default value), in the lines before you populated userroll.addroll1 and comproll.addroll2.

Vjncenzo

Answer 2

Just to give you an idea how I would have implemented it (with about 20 years of experience programming Java):

package nl.owlstead.stackoverflow;

import java.util.Random;

/*
 * Program to simulate a double die roll.
 */
public class DiceGame {
    private Random generator = new Random();

    enum Result {
        LOSER,
        TIE,
        WINNER;
    }

    public DiceGame() {
        // nothing to do in constructor
    }

    public int rollDice() {
        int zeroToFive = generator.nextInt(6);
        int oneToSix = zeroToFive + 1;
        return oneToSix;
    }

    public Result findWinner(int totalRollComp, int totalRollUser) {
        if (totalRollComp > totalRollUser) {
            return Result.LOSER;
        }

        if (totalRollComp < totalRollUser) {
            return Result.WINNER;
        }

        return Result.TIE;
    }

    public static void main(String[] args) {
        DiceGame game = new DiceGame();
        int firstRollComp = game.rollDice();
        int secondRollComp = game.rollDice();
        int totalRollComp = firstRollComp + secondRollComp;
        System.out.printf("Comp rolled: %d and %d, totalling %d%n", firstRollComp, secondRollComp,
                totalRollComp);

        int firstRollUser = game.rollDice();
        int secondRollUser = game.rollDice();
        int totalRollUser = firstRollUser + secondRollUser;
        System.out.printf("User rolled: %d and %d, totalling %d%n", firstRollUser, secondRollUser,
                totalRollUser);

        Result userResult = game.findWinner(totalRollComp, totalRollUser);
        switch (userResult) {
        case LOSER:
            System.out.println("You lost!");
            break;
        case TIE:
            System.out.println("Its a tie!");
            break;
        case WINNER:
            System.out.println("You Won!");
            break;
        }
    }
}

What is especially useful is to use as few fields as possible. As you already found out using fields (which represents state in a program) is pretty dangerous. If you can avoid state, you should do so. So in this program there is just one field: the random number generator (of course I would use new SecureRandom() instead).

Furthermore it also shows the strength of choosing good names for your variables and splitting output (System.out) and the game itself. I've tried not to introduce too many techniques beside that, but I could not resist using an enum for the result and printf to simplify the various println statements.

Answer 3

Mmmh, from a first look you are instantiating three different objects. And findwinner() is working on addroll1 = 0 and addroll2 = 0 since 0 is the default value for this class property.

If you want to work on the same data from different objects, you need to make properties static. Anyways, as other suggested, do the job on a single instance.

Answer 4

Use only one object, you are creating 3 dices and those values are not statically shared between instances...

This will work:

Dice userroll = new Dice();
userroll.userdieroll();
userroll.compdieroll();
userroll.findwinner();

Answer 5

You're creating a new Dice object each time you call a method. When you do this, you are not storing addroll1 and addroll2 in the same object, so it is only natural that .findwinner() doesn't work as expected in your third object, where you have not stored any values in addroll1 and addroll2. To fix this, use the same Dice object for all three methods, like so:

Dice tester = new Dice();
tester.userdieroll();
tester.compdieroll();
tester.findwinner();