0
\$\begingroup\$

I recently got a homework assignment from my C++ teacher. The verbatim assignment was this:

Write a program that simulates a vending machine. A customer selects and item for purchase and inserts a bill into the vending machine. The vending machine dispenses the purchased item and gives change. Assume that all item prices are multiples of 25 cents, and the machine gives all change in dollar coins and quarters. Your task is to compute how many coins of each type to return.

I'm new to C++ specifically, but I have a bit of experience with some other, higher-level languages, as well as some general experience with software development.

I took about an hour to write this code, using Google to look up certain problems I needed to overcome:

// Vending Machine Thingy (totally creative name)
// Author: [censored]
//
// This program includes all the edge cases, including handlers
//   for faulty input.
//
// Enjoy. Try to break it, if you can :)
//
// Valid breakages include infinite loops or exceptions. Early
//   exits are not breakages.

#include <iostream>  // std::cin, std::cout
#include <math.h>    // fmod
#include <array>     // std::array
#include <algorithm> // std::max_element
#include <iterator>  // std::begin, std::end
#include <limits>    // std::numeric_limits
#include <ios>       // std::streamsize

// Clears std::cin to prevent an infinite loop.
//
// This needs to happen once per input, but NOT
//   more than once
void clearcin() {
    std::cin.clear();
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
}

int main() {
    // The price of the item we're buying
    float price;

    // The bills we have available
    std::array<int, 4> bills = {1, 5, 20, 100};

    // The biggest bill we have available
    int max_bill = *std::max_element(std::begin(bills), std::end(bills));

    // The bill we're inserting into the machine
    // Must exist in `bills`
    int bill;

    while (true) {
        // Prompt the user for the price
        std::cout << "What is the price of the item you'll be buying?" << std::endl << ">";
        std::cin >> price;

        // If a non-number was entered
        if (std::cin.fail()) {
            std::cout << "The price must be a number." << std::endl;

            clearcin();

            continue;
        }

        if (fmod(price, 0.25f) != 0.0f) {
            std::cout << "The price must be a multiple of 0.25 (one quarter)." << std::endl;

            clearcin();

            continue;
        }

        if (price > max_bill) {
            std::cout << "The price must be under $" << max_bill << "." << std::endl;

            clearcin();

            continue;
        }

        if (price < 0) {
            std::cout << "Sure, let's insert a $20 bill and get $" << 20 - price << " back." << std::endl;

            clearcin();

            continue;
        }

        if (price == 0) {
            std::cout << "<3" << std::endl;

            clearcin();

            return 0;
        }

        clearcin();

        break;
    }

    while (true) {
        std::cout << "How much money will you insert into the machine?" << std::endl;

        // To get the length of the array, I'm dividing the size
        //   of the whole array by the size of one item.
        //
        // This is guaranteed to return the length of the array,
        //   since all items are guaranteed to be the same length
        //   when using `int` as the type.
        //
        // They're all pointers anyway, and pointers are always
        //   the same length, so it doesn't matter what the value
        //   is, to be honest...
        int lengthOfBills = sizeof(bills) / sizeof(bills[0]);

        // There are multiple things going on here:
        //
        // Initializer: I assign 0 to `int i`, as we're on the first
        //   element on the first iteration.
        //
        // Condition: I basically make sure `i` is below the length
        //   of the array (not equal), since we count from 0.
        //
        //   Multiplication is easier when counting from 0 :)
        //
        // After each iteration, I increment `i` by one. Nothing too
        //   interesting.
        for (unsigned int i = 0; i < lengthOfBills; i = i + 1) {
            int billValue = bills[i];

            std::cout << i + 1 << ") " << billValue << std::endl;
        }

        std::cout << ">";
        std::cin >> bill;

        if (std::cin.fail()) {
            std::cout << "Your selection must be an integer. Use the indexes on the left." << std::endl;

            clearcin();

            continue;
        }

        // The input is 1-based, we want it to be 0-based
        bill -= 1;

        if (bill < 0 || bill > lengthOfBills) {
            std::cout << "Your selection must be a valid index. Use the indexes on the left." << std::endl;

            clearcin();

            continue;
        }

        // They're both ints, so why not?
        //
        // Saves one variable :)
        bill = bills[bill];

        if (bill < price) {
            std::cout << "Try using a bigger bill! This one can't pay for the item." << std::endl;

            clearcin();

            continue;
        }

        clearcin();

        break;
    }

    // Naturally, the remainder would be
    //   `(your balance) - (the price)`
    float left = bill - price;

    // We can just chop off the extra quarters here using `floor`
    int dollars = floor(left);

    // Take the remainder of `left / 4`, multiply it by 4, and
    //   convert to an int using `floor`
    int quarters = floor(fmod(left, 1) * 4);

    std::cout
        << "You have $" << left << " left." << std::endl
        << "The machine dispenses $" << dollars << " and " << quarters << " quarter" << (quarters == 1 ? "" : "s") << "." << std::endl;

    if (dollars > 20) {
        std::cout << "Wow, that's a lot of change. I hope there's a Coinstar nearby." << std::endl;
    }

    // `return 0` is implicit
}

I even included a challenge at the top for my teacher to break it. Excuse all the comments, please. He told me last time that I didn't include enough comments, so I decided to play it safe here.

I'm interested in learning how this code can be improved. Namely, I'm a bit skeptical about all the calls to clearcin(), because they're scattered all over the program. Any feedback is appreciated, I'd love to know what you think of it!

P.S. 0 is an easter egg. All behavior there is intentional. :P

\$\endgroup\$
  • \$\begingroup\$ Your code doesn't really solve the challenge. It is specifically stated the customer selects an item, then inserts the payment. \$\endgroup\$ – tinstaafl Sep 25 '18 at 23:39
  • \$\begingroup\$ You select the item by setting the price. Inserting the payment is choosing between 4 bills. How does this not solve the challenge? It's not exactly specified that there have to be a fixed set of items, so allowing the user to choose their own price should be fine, right? \$\endgroup\$ – Dev Sep 25 '18 at 23:43
  • 1
    \$\begingroup\$ Not really. different items could have the same price, at least in any vending machine I've seen. How does the machine know which item if it only knows the price? \$\endgroup\$ – tinstaafl Sep 25 '18 at 23:47
  • 1
    \$\begingroup\$ Just for future reference, these kinds of challenges are very specific and they expect the code to specifically follow what it says. Taking shortcuts to work around the more onerous parts of the challenge will not gain any points. \$\endgroup\$ – tinstaafl Sep 25 '18 at 23:54
  • 1
    \$\begingroup\$ You say, you are playing safe. The interpretation by some reader might be that you are childish. If you want to be taken seriously this is not a good start. \$\endgroup\$ – stefan Sep 26 '18 at 7:26
1
\$\begingroup\$

Be careful using floating-point arithmetic for money

The challenge has been favourable to you, because all your prices are a multiple of 0.25, which can be represented exactly in binary. If the environment changes, so that prices are in units of 0.05, for instance, code such as the fmod() test won't necessarily do what you want:

    if (fmod(price, 0.25f) != 0.0f) // works, for "small" price

    if (fmod(price, 0.05f) != 0.0f) // may have rounding error

It's better to represent currency using one of the integer types; you'll need to parse the dollars and cents separately and combine them into total cents in your code.

Bills

I like the bills array, that gives you a single point of control for the inputs that will be accepted (making it easier to change for different currencies, for example). It may be to your advantage to have a similar array for the coins that can be given in change.

One improvement that can be made is that bills and max_bill don't need automatic storage, as they are constant:

static const std::array<int, 4> bills = {1, 5, 20, 100};

// The biggest bill we have available
static const int max_bill = *std::max_element(std::begin(bills), std::end(bills));

static const std::size_t lengthOfBills = sizeof bills / sizeof bills[0];

Given that bills is a std::array, we should just be using its size() member instead:

    for (std::size_t i = 0;  i < bills.size();  ++i)

Similarly, if we know it's sorted, we can use bills.back() to get the largest value.

Coins

Unlike the bills, the coin handling is very inflexible, and hard to change if you add additional coins to the machine's repertoire.

Other improvements

  • Include <cmath> rather than <math.h> and qualify names e.g. std::fmod(), std::floor().
  • Signed/unsigned mismatch in for (unsigned int i = 0; i < lengthOfBills; i = i + 1) - lengthOfBills should probably be a std::size_t.
  • Although the challenge specifies that the customer inserts "a bill" and receives change, it would be nicer if it would accept multiple bills as necessary (so the customer could insert 3×$1 for an item costing 2.25).
  • It's more idiomatic to write !std::cin than std::cin.fail().
\$\endgroup\$
  • \$\begingroup\$ This is it? Wow, I thought my program would be way worse! :P Well, thanks for your feedback! \$\endgroup\$ – Dev Sep 26 '18 at 10:54
  • \$\begingroup\$ I'm not really an expert in interactive I/O, so I'm leaving it to someone more knowledgeable to suggest improvements to your clearcin() and its usage... \$\endgroup\$ – Toby Speight Sep 26 '18 at 10:59
  • \$\begingroup\$ I don't think clearcin() itself is an issue - I'm more worried about the DRY issues in all the calls to it. \$\endgroup\$ – Dev Sep 26 '18 at 11:00
  • \$\begingroup\$ Also, I wasn't aware that cmath existed. This is one of my first C++ programs. Thanks for letting me know about it. I do use qualified names everywhere else - the teacher encourages use namespace std, but, kindly, hell no. :) \$\endgroup\$ – Dev Sep 27 '18 at 5:03

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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