8
\$\begingroup\$

While I was writing this I had some thoughts and questions going through my head about if I was doing this in a good manner. For instance, is it good to use a StringTokenizer to loop through the words in the line I copied from the Scanner's input?

Write a method named coinFlip that accepts as its parameter a Scanner for an input file. Assume that the input file data represents results of sets of coin flips that are either heads (H) or tails (T) in either upper or lower case, separated by at least one space. Your method should consider each line to be a separate set of coin flips and should output to the console the number of heads and the percentage of heads in that line, rounded to the nearest tenth. If this percentage is more than 50%, you should print a "You win" message.

public static void coinFlip(Scanner input) {
    
    int countHeads = 0;
    int countTails = 0;
    double count = 0.0;
    double ratio = 0.0;
    String line = "";
    
    while(input.hasNextLine()) {
        countHeads = 0;
        countTails = 0;

        line = input.nextLine();
        StringTokenizer lineTokens = new StringTokenizer(line);
        while (lineTokens.hasMoreTokens()) {
            String token = lineTokens.nextToken();
            if (token.toLowerCase().equals("h")) {
                countHeads++;
            } else {
                countTails++;
            }
        }

        count = countTails + countHeads;

        ratio = Math.abs(1 - (countTails/count)) * 100;

        System.out.printf(countHeads + " heads (%1.1f%%)\n", ratio);

        if (countHeads > countTails) {
            System.out.println("You win!\n");
        }
        else {
            System.out.println();
        }
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ Would string.split() be better? This would give you the total as the array length and you would only have to increment one counter. \$\endgroup\$ Commented Dec 4, 2015 at 20:17
  • \$\begingroup\$ @Brianfromstatefarm I don't recommend String.split() because its behaviour is tricky. \$\endgroup\$ Commented Dec 4, 2015 at 20:35

4 Answers 4

5
\$\begingroup\$

The biggest simple improvement you could make would be to declare your variables in a more limited scope. As you have written it, I cannot easily tell whether the statistics are cumulative, or each line is being treated as an independent problem.

count being a double is weird; it should be an int.

I suggest calling .toLowerCase() once per line instead of once per word.

It's probably worthwhile to do an extra simple check to ensure that every word that is not an "H" is a "T".

The calculation for ratio is more complicated than it needs to be.

You aren't using printf() effectively — the whole point of printf() is to take advantage of the format string rather than concatenation.

public static void coinFlip(Scanner input) {
    while (input.hasNextLine()) {
        int heads = 0, tails = 0;

        StringTokenizer tokens =
            new StringTokenizer(input.nextLine().toLowerCase());
        while (tokens.hasMoreTokens()) {
            String token = tokens.nextToken();
            if ("h".equals(token)) {
                heads++;
            } else if ("t".equals(token)) {
                tails++;
            }
        }

        double ratio = 100.0 * heads / (heads + tails);
        System.out.printf("%d heads (%1.1f%%)\n", heads, ratio);
        System.out.println((heads > tails) ? "You win!\n" : "");
    }
}

One alternative to StringTokenizer would be to use Scanner.findInLine().

public static void coinFlip(Scanner input) {
    while (input.hasNextLine()) {
        int heads = 0, tails = 0;
        String token;
        while (null != (token = input.findInLine("\\b[HhTt]\\b"))) {
            switch (token) {
                case "H": case "h": heads++; break;
                case "T": case "t": tails++; break;
            }
        }

        double ratio = 100.0 * heads / (heads + tails);
        System.out.printf("%d heads (%1.1f%%)\n", heads, ratio);
        System.out.println((heads > tails) ? "You win!\n" : "");
    }
}
\$\endgroup\$
4
\$\begingroup\$
    ratio = Math.abs(1 - (countTails/count)) * 100;

looks a bit convoluted. First, is there a possibility for 1 - (countTails/count) to become negative? Second,

    ratio = 100 * countHeads/count;

looks more straightforward.

Having written it this way, you may change the type of count to a natural int (I must say it took me some time to understand that division you have is not an integer):

    ratio = (100.0 * countHeads)/count;
\$\endgroup\$
2
  • \$\begingroup\$ count is countTails + countHeads, so countTails/count will never be bigger than 1. This means that you'll never have a negative number. \$\endgroup\$
    – Nzall
    Commented Dec 4, 2015 at 23:17
  • \$\begingroup\$ @NateKerkhofs The question was meant for OP to answer :) \$\endgroup\$
    – vnp
    Commented Dec 4, 2015 at 23:31
3
\$\begingroup\$

One tiny little thing that's almost too minor to even mention:

if (token.toLowerCase().equals("h")) {
    countHeads++;
} else {
    countTails++;
}

is inconsistent with:

if (countHeads > countTails) {
    System.out.println("You win!\n");
}
else {
    System.out.println();
}

I believe } else { is the more idiomatic indenting style of the two. Doesn't matter which one you use, really - but being consistent does.


You could do away with the final else block here:

if (countHeads > countTails) {
    System.out.println("You win!\n");
}
else {
    System.out.println();
}

Simply by removing the \n from the output string:

if (countHeads > countTails) {
    System.out.println("You win!");
}
System.out.println();
\$\endgroup\$
1
\$\begingroup\$

Besides the 'legacy' StringTokenizer class or the String.split() method suggested by @Brianfromstatefarm's comment, you can also consider parsing each line using the Scanner class.

Since you are already using System.out.printf() for your output, you can rely less on explicit concatenation and repeated method calls, and rely more on the format modifiers. For example, it can be rewritten as such:

// System.out.printf(countHeads + " heads (%1.1f%%)\n", ratio);

// if (countHeads > countTails) {
//     System.out.println("You win!\n");
// }
// else {
//     System.out.println();
// }

System.out.prinf("%d heads (%1.1f%%)\n%s\n", countHeads, ratio, 
                    countHeads > countTails ? "You win!" : "");
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.