Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

So... I have a program in which I want to flip heads three times in the row.

What I'm asking for is for proposals of other solutions for this program, in pro way, as You do in natural sense.

That's my code as Java novice.

/*
 * File: ConsecutiveHeads.java
 * ----------------
 * This program flips a coin repeatly until three consecutive heads
 * are tossed.
 */

import acm.program.*;
import acm.util.*;

public class ConsecutiveHeads extends ConsoleProgram {
    /* Run the program */
    public void run() {
        println("This program flips a coin until there are three" +
                "heads in the row.");
        while(counter != 3) {
            FlipACoin();
        }
        println("Yupii! There are already three same heads in the row :)");
    }

    /* Flip a coin. Then if heads are tossed, increment our counter.
     * In tails case, zero counter. */
    private void FlipACoin() {
        boolean rank = rgen.nextBoolean();
        if(rank) {
            println("heads");
            counter++;
        } else {
            println("tails");
            counter = 0;
        }
    }

    /* Create an instance variable for the random number generator */
    private RandomGenerator rgen = new RandomGenerator();

    /* Create an instance variable for counter detecting three heads in row */
    private int counter = 0;
}
share|improve this question
    
Unless you need cryptographic stength randomness, this approach is good. –  Johan Sjöberg Feb 19 '11 at 17:01
add comment

migrated from stackoverflow.com Feb 19 '11 at 17:10

This question came from our site for professional and enthusiast programmers.

3 Answers

Your function is called FlipACoin() but you are doing more than just flipping the coin.
A more appropriate name might be FlipACoinAndCountConsecutiveHeadRolls().

I'd suggest keeping the function as simple as the name suggests and return a string (or bool) from FlipACoin(), then examine the result outside the function.
This will also allow you to make the counter a local variable to the run function, which will make the class smaller.

share|improve this answer
add comment

Some things you could consider

  • make the starting class a simple program which calls other methods and/or classes to do most of the work.
  • if you want something more general, you could use an abstract output stream so it can be captured or even use a Listener interface.
  • make fields which are not intended to change final this can improve clarity.
  • is it conventional to place fields then constructors then methods as the class order.
  • you could restructure the fields/method calls to be thread safe if that might be an issue. e.g. SimpleDateFormat is not thread-safe and causes no end of concern.
share|improve this answer
1  
I can tell you're unfamiliar with the ACM Library. These suggestions would make perfect sense, but they don't apply to ACM. ConsoleProgram is its own program. ACM hides the main method because it introduces too many complex concepts to new programmers. Also not supposed to call its constructor. RandomGenerator is a class in ACM. They didn't like the the Random class because the name makes it unclear that its a number generator. (I think they possibly changed the implementation to make it more random.) –  Eva Jan 16 '13 at 20:37
    
@Eva Thank you for the clarification. It can be interesting to read old answers to see if I would write them differently today and you comment suggests some improvements. –  Peter Lawrey Jan 16 '13 at 21:24
add comment

I'd do it like this:

/*
 * File: ConsecutiveHeads.java
 * ----------------
 * This program flips a coin repeatly until three consecutive heads
 * are tossed.
 */

import acm.program.*;
import acm.util.*;

public class ConsecutiveHeads extends ConsoleProgram {
    /* Run the program */
    public void run() {
        println("This program flips a coin until there are three" +
                "heads in the row.");
        while(!TryFlipThreeHeads()) {
        }
        println("Yupii! There are already three same heads in the row :)");
    }

    /* Flip a coin. */
    private boolean FlipACoin() {
        boolean rank = rgen.nextBoolean();
        if(rank) {
            println("heads");
        } else {
            println("tails");
        }
        return rank;
    }

    private boolean TryGetThreeHeads() {
         for(int counter = 0; counter < 3; counter++)
         {
              // if we get a tail, give up
              if(!FlipACoin()) return false;
         }
         return true;
    }

    /* Create an instance variable for the random number generator */
    private RandomGenerator rgen = new RandomGenerator();
}

That way each piece is self-contained. FlipACoin just flips the coin, it doesn't keep track of counting. And TryGetThreeHeads() only worries about trying to flip three heads not the process of flipping each coin.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.