Calculating the number of candy bars and gumballs that can be redeemed using coupons

Question

The following question was taken from Absolute Java 5th ed. by Walter Savitch:

The video game machines at your local arcade output coupons according to how well you play the game. You can redeem 10 coupons for a candy bar or 3 coupons for a gumball. You prefer candy bars to gumballs. Write a program that defines a variable initially assigned to the number of coupons you win. Next, the program should output how many candy bars and gumballs you can get if you spend all of your coupons on candy bars first, and any remaining coupons on gumballs.

This is the code that I have written:

public class Question2 {

    private static final int NUMBER_OF_COUPONS_FOR_A_CANDYBAR = 10;
    private static final int NUMBER_OF_COUPONS_FOR_A_GUMBALL = 3;

    private static int numberOfCoupons = 13;

    public static void main(String[] args) {

        System.out.println(numberOfCandybars(numberOfCoupons)
                + " candybar(s) and " + numberOfGumballs(numberOfCoupons)
                + " gumball(s).");

    }

    private static int numberOfCandybars(int initialCoupons) {
        return initialCoupons / NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
    }

    private static int numberOfGumballs(int remainingCoupons) {
        remainingCoupons = numberOfCoupons % NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
        return remainingCoupons / NUMBER_OF_COUPONS_FOR_A_GUMBALL;
    }
}

Answer 1

Looking at this statement, it looks as if numberOfGumballs and numberOfCandybars are independently derived from numberOfCoupons:

System.out.println(numberOfCandybars(numberOfCoupons)
        + " candybar(s) and " + numberOfGumballs(numberOfCoupons)
        + " gumball(s).");

But they are not independent, because numberOfGumballs actually depends on numberOfCoupons and numberOfCandybars too. Hiding this fact is not a good thing: one might expect that changing the behavior of numberOfCandybars won't affect the behavior of numberOfGumballs, but it actually will, which is confusing.

If you think about it, it would be best if numberOfCandybars returned 2 values:

• The number of candy bars our coupons could buy
• and, the number of coupons left

With these two values, we could use the number of coupons left to make another call to numberOfGumballs, and keep the number of candy bars for printing later. That way, the relationships between the functions could be well visible.

Of course, Java can only return a single value. But the single value can be an object, which can contain multiple values.

So let's start over. We have items: Candy bar, Gumball. We have some kind of a vending machine, that you put coupons into, select the item you want, and it gives back a number of items and change (remaining coupons).

Let's model the items with an enum, and give them a price too.

enum Item {
    CandyBar(10),
    Gumball(3);

    final int price;

    Item(int price) {
        this.price = price;
    }
}

And let's model the machine and the output it gives:

class AutomaticCandyMachineOutput {
    final Item item;
    final int itemCount;
    final int change;

    AutomaticCandyMachineOutput(Item item, int itemCount, int change) {
        this.item = item;
        this.itemCount = itemCount;
        this.change = change;
    }
}

class AutomaticCandyMachine {
    AutomaticCandyMachineOutput redeemCouponsForItems(int coupons, Item item) {
        int itemCount = coupons / item.price;
        int change = coupons % item.price;
        return new AutomaticCandyMachineOutput(item, itemCount, change);
    }

    AutomaticCandyMachineOutput redeemCouponsForCandybars(int coupons) {
        return redeemCouponsForItems(coupons, Item.CandyBar);
    }

    AutomaticCandyMachineOutput redeemCouponsForGumballs(int coupons) {
        return redeemCouponsForItems(coupons, Item.Gumball);
    }
}

This seems a more natural, flexible and extensible design. Now we can rewrite the original statement like this:

int coupons = 99;
AutomaticCandyMachine machine = new AutomaticCandyMachine();
AutomaticCandyMachineOutput output1 = machine.redeemCouponsForCandybars(coupons);
AutomaticCandyMachineOutput output2 = machine.redeemCouponsForGumballs(output1.change);

String.format(
    "%d candybar(s) and %d gumball(s)",
    output1.itemCount,
    output2.itemCount);

Answer 2

• Don't keep global state and don't make everything static: That makes your application rigid and more difficult to unit test. Either make the static methods for calculations more generic math functions or make them part as Question2's object behavior
• Question2 isn't a Question2 in this case. It's more a convenient placeholder for your static void main method. It's more a Question2Resolver.
• Make values that never change final, that way you can be sure that when a bug happens, those values are always correct. As a future reader, knowing these are final spares them the headache of tracking down behavior that might change them.
• Format Strings using String.format() for readability. Not always better, but often is.
• Never assign values to parameters as you did with remainingCoupons. Future readers of your code can easily miss nuances like that. Keep it clear, declare a seperate variable for that.

public class Question2Resolver {

    private static final int NUMBER_OF_COUPONS_FOR_A_CANDYBAR = 10;
    private static final int NUMBER_OF_COUPONS_FOR_A_GUMBALL = 3;

    private final int numberOfCoupons;

    public Question2Resolver(int numberOfCoupons) {
        this.numberOfCoupons = numberOfCoupons;
    }

    public static void main(String[] args) {
        Question2Resolver question2Resolver = new Question2Resolver(13);
        System.out.println(String.format("%s candybar(s) and %s gumball(s).",
                question2Resolver.numberOfCandybars(),
                question2Resolver.numberOfGumballs()));
    }

    private int numberOfCandybars() {
        return numberOfCoupons / NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
    }

    private int numberOfGumballs() {
        int remainingCoupons = numberOfCoupons
                % NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
        return remainingCoupons / NUMBER_OF_COUPONS_FOR_A_GUMBALL;
    }
}

You could make the algorithm more robust, by giving numberOfGumballs a parameter which take the result of numberOfCandybars.

Answer 3

numberOfGumballs

Why does this calculate your remaining coupons? It should strictly be for calculating the amount of Gumballs you get in order to allow a more general purpose usage of the application.

private static int numberOfGumballs(int remainingCoupons) {
    remainingCoupons = numberOfCoupons % NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
    return remainingCoupons / NUMBER_OF_COUPONS_FOR_A_GUMBALL;
}

Should (in my opinion) become:

Purely find out how many gumballs you could get with your coupons.

//For question 2: Before calling this, reduce the amount of coupons you have.
private static int numberOfGumballs(int coupons) {
    return remainingCoupons / NUMBER_OF_COUPONS_FOR_A_GUMBALL;
}

OR

Redeem as many gumballs as possible with your coupons.

Note: You cannot call .remove on integers, so you'd have to save your coupons in some other way, or pass them in such a way so you can modify the value.

private int redeemGumballs(int coupons) {
    int gumballs = coupons/NUMBER_OF_COUPONS_FOR_A_GUMBALL;
    int coupons_used = gumballs*NUMBER_OF_COUPONS_FOR_A_GUMBALL;
    coupons.remove(coupons_used);
    return gumballs;
}

If you pick this latter one, numberOfCandybars should also reflect the requirement of reducing the tickets you have left after redeeming.

private int redeemCandybars(int coupons) {
        int candybars = coupons/NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
        int coupons_used = candybars*NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
        coupons.remove(coupons_used);
        return candybars;
}