Java Coin Toss

awney

So, I am trying to get this small piece of code working for an assignment in uni - here's the brief ... You are developing code for a computer game called CoinSnap, which involves tossing three coins.
Write a Java class representing a single coin, with public methods toss (randomly selects heads or tails), isHeads (returns true if the coin is heads), and toString (returns a string “Heads” or “Tails”). The class may also require one or more private member variables.

by the way we're currently on the topic of inheritance so that's why i have spread the code over two classes rather than have it all in one ...

here's what i have -

parent class:

import java.util.Random;
import javax.swing.JOptionPane;

public class Assignment6CoinSnap {

private static int side;
private static boolean isHeads;

public int toss()
{
Random Flip = new Random();
int side = Flip.nextInt(2);
return side;
}

public boolean isHeads()
{return isHeads = ((side == 1) ? false : true);}


public String toString(){
String message;
if (isHeads == true){ message = "Heads";}
else{message = "Tails";}
return message;


}
}

then the child class:

import javax.swing.JOptionPane;

public class Assignment6CoinSnapTest {
public static void main(String[] args)
{

Assignment6CoinSnap Snap = new Assignment6CoinSnap();

for (int i = 0; i<3; i++){
Snap.toss();
Snap.isHeads();

String result = Snap.toString();
JOptionPane.showMessageDialog(null,"Result is " + result);

}
}

}

but every time i run that code it just comes up as Heads - i've even checked it with 100 iterations ... what have i done wrong?!

Ballyv24

I would get rid of the call to Snap.isHeads(); and also delete the isHeads method from the Assignment6CoinSnap class. This is making it more complicated and is not necessary. (In my opinion)

Then in the toss() method set the value of the boolean isHeads to true or false, depending on the value of side

Hope that helps

awney

Ballyv24 wrote: »

I would get rid of the call to Snap.isHeads(); and also delete the isHeads method from the Assignment6CoinSnap class. This is making it more complicated and is not necessary. (In my opinion)

i can see how that would make it easier but we're told to use three methods, toss, isHeads and toString ... i think they're trying to make it as complicated as possible! it is definitely in there that my problem is ...

Ballyv24

awney wrote: »

int side = Flip.nextInt(2);

Hi, the issue turns out not to be too bad, but it is one of those issues that you could spend hours looking at and not figure out. (It happens to me too often)

change the quoted line to

side = Flip.nextInt(2);

The way you currently have it, you had 2 values in the Assignment6CoinSnap class called side. One existed for the full class, the other only existed within the toss() method. The random number was being assigned to the "side" in the toss() method.

Hope that works for you

JazzyJ

Haven't written java in years - usually C# - but I've a few comments:

• You're not using inheritance here. You just calling a class from another class.
• There's no reason for your side and isHeads variables to be static. In the current case you can't have multiple coins as those variables would be the same across all of them. It would be beneficial in the case of multiple coins being flipped at the same time.
• The following link recommends using nextBoolean() where appropriate http://www.javamex.com/tutorials/random_numbers/java_util_random.shtml
• Not sure of the exact problem with it returning heads every time but it could possibly be due to instantiating a Random object every time toss is called. You could make the Random class a private variable instantiate it in the constructor and only get the next value each time toss is called.
• Just seen Ballyv24's response - good practice when referencing class variables is to prefix them with "this.", i.e. this.side = Flip.nextBoolean();

Aswerty

I think Ballyv2 has spotted the main culprit and JazzyJ has pointed out most other issues. I would point out that the way you have implemented your Assignment6CoinSnap class means that you don't actually need to return a value from the toss and isheads methods since they modify the private variables which are then accessed by the toString class. These methods return types would therefore be void and the return keyword would not be required. The toString method is fine since do you want to return a value. Your current implementation will not be an issue but it does mean you have some redundant, and probably undesirable, functionality.

croo

What Ballyv24 says is correct.. I would change to

public int toss() {
		Random Flip = new Random();
		this.side = Flip.nextInt(2);
		return side;
	}

I would get rid of the call to Snap.isHeads();

I would disgree and actually do the opposite here this this. There is no need for the isHeads variable at all, simple call the modified isHeads() method

public boolean isHeads() {
		return ((side == 1) ? false : true);
	}

if you make this change you would need to change the check in the toString() method

if (isHeads() == true) {
			message = "Heads";
		} else {
			message = "Tails";
		}
		return message;

But, again, I would say the message variable adds no real value. So, I would drop it and just return the true or false based on the isHeads() result

public String toString() {
		if (isHeads()) 
			return "Heads";
		else 
			return "Tails";
	}

Though, if I'm honest on another day I might like the idea of having just 1 return!

For a similiar reason I would say there is no need for the toss method to return side... you are setting it in the method... and you don't even check it's value anyway! So I would use

public void toss() {
		Random Flip = new Random();
		this.side = Flip.nextInt(2);
	}

And again in the main of the test class you use another variable that adds no value

String result = Snap.toString();
JOptionPane.showMessageDialog(null,"Result is " + result);

Just skip the result and go straight to the toString!

for (int i = 0; i < 3; i++) {
			Snap.toss();
			JOptionPane.showMessageDialog(null, "Result is " + Snap.toString());
		}

Finally in the main of Test you call isHeads() after the toss() and again don't even check its value. You just don't need to make this call and it adds nothing. See the above loop with the call to isHeads() removed.

It might be a bit dated now but I think Martin Fowler's classic "Refactoring" is a great read for helping write clean code - or revisiting and refactoring code written to tidy it up after the initial draft. Maybe you can find it in the uni's library! Highly recommended.

awney

Ballyv24 wrote: »

Hi, the issue turns out not to be too bad, but it is one of those issues that you could spend hours looking at and not figure out. (It happens to me too often)

change the quoted line to

side = Flip.nextInt(2);

The way you currently have it, you had 2 values in the Assignment6CoinSnap class called side. One existed for the full class, the other only existed within the toss() method. The random number was being assigned to the "side" in the toss() method.

Hope that works for you

THANK YOU - THANK YOU - THANK YOU!!
i don't think i would have slept tonight without that being solved ... you're a legend!

SionnachRoe

What is the exercise trying to test?

It looks like
i. Defining classes which represent real world objects (a coin)
ii. Give them some state/functionality which will differ between the instances (heads/tails)
iii. Create and use multiple instances (3 coins)

If you are covering inheritance and classes then the above seem reasonable.

Inheritance is not needed for this (except in the most trivial sense of overriding toString()) but there should be a Coin class and thus a separate Coin file.

import java.util.Random;
 
public class Coin {
 
 private static Random rnd = new Random();
 
 private boolean heads;
 
 public void toss(){
  heads = rnd.nextBoolean();
 }
 
 public boolean isHeads(){
  return heads;
 }
 
 @Override public String toString(){
  return heads
    ? "Heads"
    : "Tails";
 }
}

NOTES:

heads is an instance variable, it needs to be different for each instance.
rnd can be shared across all the instances (static).

toString() is a member of the Object class and we want to override it (inheritance related) to give the Heads/Tails result.


To test/demonstrate the Coin class we can create an array of them and toss each one, displaying the result

[LEFT]public class BoardsProblem {[/LEFT]
 
[LEFT]  public static void main(String [] args){[/LEFT]
 
[LEFT]      Coin[] coins = new Coin[]{
          new Coin(),
          new Coin(),
          new Coin(),
      };[/LEFT]
 
[LEFT]      for(Coin coin : coins){
          coin.toss();
          System.out.println("The coin shows " + coin.toString()); 
      }
  }[/LEFT]
}
 

The first section sets up an array of coins (see http://www.codeguru.com/java/tij/tij0053.shtml )

The loops through the array tossing each coin and displaying the result.

hth,
Alan.

JazzyJ

^^^^^^^^^^^


</thread>



OT: First post after being a member 3 years? I thought I was bad