Constructor create instance, and that constructor create another instance

[Nils Anton]

Hello!

The task I'm attempting to solve is part of a school assignment. I've already solved it one way - yet that solution is rather ugly - so I'm attempting to solve it another way.

The task requires me to create a new class PairOfDice that creates two Dice objects, rolls them, checks if they're a pair, and then writes the result. I'm trying to have the PairOfDice class create two Dice classes that roll themselves, save their value to themselves, and I then have the PairOfDice class check if they rolled a pair, and lastly write out the result. Yet when I attempt to have the PairOfDice class create two Dice objects, they are created with a wrong number of sides (value), yet I don't understand why.

I've attempted to search this forum, and the Internet, yet I still don't understand what the error is - would you please help me?

Dice class:

public class Dice {
  private int nSides;
  private int value;
  
  public Dice() {
    this(6);
  }
  
  public Dice(int nSides) {
    if (nSides<2) {
      System.out.println("Wrong number of sides");
      this.nSides = 6;
    } 
    else {
      this.nSides = nSides;
    }
    this.roll();
  }
  
  public int getValue() {
    return this.value;
  }
  
  public void roll() {
    this.value = (int)(Math.random()*this.nSides) + 1;
  } 
 
  public String toString() {
    return "Dice(" + this.nSides + ", " + this.value + ")";
  }
}

PairOfDice class:

public class PairOfDice {
  
  private int nsides;
  
  Dice dice01;
  Dice dice02;
  
  private int[] arrayValues = new int[2];
  
  public PairOfDice() {
    this(6);
  }
  
  public PairOfDice(int nSides) {
    Dice dice01 = new Dice(nsides);
    Dice dice02 = new Dice(nsides);
  }
  
  public int[] getValues() {
    this.arrayValues[0] = this.dice01.getValue(); 
    this.arrayValues[1] = this.dice02.getValue();
    return arrayValues;
  }
  
  public boolean isPair() {
    if (this.arrayValues[0] == this.arrayValues[1]) {
      return true;
    }
    
    else {
      return false;
    }
  }
  
  public String toString() {
    return dice01.toString() + dice02.toString();
  }
}

PairOfDiceTestRun class:

public class PairOfDiceTestRun {
  
  boolean isPair = false;
  
  public static void main (String[] args) {
    PairOfDice pairOfDice = new PairOfDice();
    if (pairOfDice.isPair() == true) {
      System.out.print(pairOfDice.toString() + "\t Pair!");
    }
    
    else {
      System.out.print(pairOfDice.toString());
    }
  }
}

/Regards,
Nils

EDIT: I've done some debugging - when I run:

Dice dice01 = new Dice(nsides);

dice01 gets the value:
dice01 Dice(6, 2) Dice

which I assume has something to do with toString - does it automatically rewrite dice01's value when it's created?

[Dave Tolls]

So what do you see from running this and what do you expect to see?

[Dave Tolls]

Just seen your edit.
Why are you expecting something other than a 6 for the number of sides?

[Henry Wong]

First, welcome to the ranch...

Hint: What is the difference between declaring an instance variable versus declaring a local variable? And what do you thinks happens when you do both?

Henry

[Dave Tolls]

Ah, and now I see why I didn't spot that, as I would have expected a NullPointerException...except the code that populates the int[] isn't actually called.

[Nils Anton]

@Dave: I think it displays what the Dice rolled - so in this case 2.

@Henry: local variables don't get default values. Instance variables get default values. If they're created at the same time - would the instance variable then overwrite the local variabel?

I just also found another error in the code: the "nsides" variabel in PairOfDice should be "nSides".

Also, thanks for the welcome .

[Campbell Ritchie]

Welcome to the Ranch

I suggest you avoid Math.random(). I know the Java® Tutorial say it is more convenient than a Random instance, but I am not convinced. Using the nextInt method is more robust than the (int) cast.

import java.util.Random;
// ...
Random myRandom = new Random();
// ...
public final int roll()
{
   return myRandom.nextInt(sides) + 1;
}

You should mark all method called from the constructor private or final; there is some discussion about that problem here.

Never write == true or == false. Not
if (b == true) ... but if (b) ...
Not
if (b == false) ... but if (!b) ...
Using == is error‑prone; we see people who write = by mistake.
Don't write
if (...) return true; else return false;
You should write

public boolean isPair()
{
   return dice1.getValue() == dice2.getValue();
}

You can simplify the method which returns the array

public int[] getValues()
{
   return new int[]{dice1.getValue(), dice2.getValue()};
}

Now you no longer need the array field at all.

What you are getting back from the printing call is determined by the toString method which you wrote. It is confusing because you don't say what the numbers mean. If you wrote this as your toString method, it would be easier to understand:-

@Override
public String toString()
{
   return String.format("A %d-sided die showing %d.", nsides, value);
}

That will give a very similar output to
"A " + nsides + "-sided die showing " + value + "."

[Nils Anton]

Thank you for the extensive help!

[Campbell Ritchie]

Our pleasure