5
\$\begingroup\$

Here's the school assignment I did:

Problem 1. Write a program that simulates a soft drink machine.

The program should use a structure Beverage that stores the following data:

  • Drink Name
  • Drink Cost
  • Number of Drinks in Machine

The program should create an array of five structures. The elements should be initialized with the following data:

$$\begin{array}{l|r|c} \textrm{Drink Name} & \textrm{Cost} & \textrm{Number in Machine} \\ \hline \textrm{Cola} & 0.75 & 20 \\ \textrm{Root Beer} & 0.75 & 20 \\ \textrm{Lemon-Lime} & 0.75 & 20 \\ \textrm{Grape Soda} & 0.80 & 20 \\ \textrm{Cream Soda} & 0.80 & 20 \\ \end{array} $$

Each time the program runs, it should enter a loop that performs the following steps:

  1. A list of drinks is displayed on the screen.
  2. The user should be allowed to either quit the program or pick a drink.
  3. If the user selects the drink, he or she will next enter the amount of money that is to be inserted into the drink machine.
  4. The program should display the amount of change that would be returned and subtract one from the number of that drink left in the machine.
  5. If the user selects a drink that had sold out, a message should be displayed. Similarly, if the user does not specify enough money, a different message should be displayed (and the drink should not be dispensed).

When the user chooses to quit the program, it should display the total amount of money the machine earned.

And here's my code:

#include <stdio.h>
#include <string>
#include <iostream>
#include <iomanip>
struct Beverage {
    std::string drinkName;
    const double drinkCost;
    unsigned int numberOfDrinks;

};
struct DrinkMachine {
    Beverage * drinks;
    double revenue;
};
void initialDrinkMachinePrompt(Beverage drinks[], int numOfDrinks);
void promptDrinkMachineMenu(int numOfDrinks);
char getDrinkMachineMenuResponse(int numOfDrinks);
bool isValidDrink(char response, int numOfDrinks);
void displayDrinks(Beverage drinks[], int numOfDrinks);
void displayListNumberedItem(int itemNumber);
int findMaxDrinkNameStrLength(Beverage drinks[], int numOfDrinks);
int findMaxNumOfDrinksStrLength(Beverage drinks[], int numOfDrinks);
void displayDrink(Beverage drink, int nameStrLength, int costStrLength, int numOfDrinksStrLength);
int getUserMoney();
void promptUserForMoney();
double purchaseDrink(Beverage drinks[], int drinkIndex, double userMoney);
bool isDrinkSoldOut(Beverage drink);
bool hasEnoughMoney(Beverage drink, double userMoney);
double displayChange(double cost, double userMoney);
void closeDrinkMachine(DrinkMachine dm);

const int ONE_SPACE_PADDING = 1;
const int DRINK_COST_PRECISION = 2;
const int DRINK_COST_MAX_STR_LENGTH = 5;
const int INDEX_TO_NON_ZERO_LIST = 1;
const char QUIT_CHARACTER = 'q';
int main(int argc, char *argv[]) {
    const int NUM_OF_DRINKS = 5;
    DrinkMachine dm = {
        new Beverage [NUM_OF_DRINKS] {
            {"Cola", 0.75, 0},
            {"Root Beer", 0.75, 20},
            {"Lemon-Lime", 0.75, 20},
            {"Grape Soda", 0.80, 20},
            {"Cream Soda", 0.80, 20}}
        , 0 };
    char userResponse = 'y';
    std::cout << "Welcome to the Drink Machine" << std::endl;
    while(true) {
        initialDrinkMachinePrompt(dm.drinks, NUM_OF_DRINKS);
        userResponse = getDrinkMachineMenuResponse(NUM_OF_DRINKS);
        if(userResponse == QUIT_CHARACTER) break;
        double userMoney = getUserMoney();
        dm.revenue += purchaseDrink(dm.drinks, userResponse-'0'-1, userMoney);
    }
    closeDrinkMachine(dm);
}

void closeDrinkMachine(DrinkMachine dm) {
    std::cout << "Thank you for using the drink machine. \n"
    << "The drink machine has earned: " << dm.revenue << std::endl;
 }
double purchaseDrink(Beverage drinks[], int drinkIndex, double userMoney) {
    if(isDrinkSoldOut(drinks[drinkIndex])) {
        std::cout << "Sorry " << drinks[drinkIndex].drinkName
        << " is currently sold out. Try another drink." << std::endl;
        return 0;
    } else if(!hasEnoughMoney(drinks[drinkIndex], userMoney)) {
        std::cout << "Sorry you don't have enough money." << std::endl;
        return 0;
    } else {
        --drinks[drinkIndex].numberOfDrinks;
        displayChange(drinks[drinkIndex].drinkCost, userMoney);
        return drinks[drinkIndex].drinkCost;
    }
}
double displayChange(double cost, double userMoney) {
    std::cout << "Here's your change " << userMoney - cost << "." << std::endl;
    return userMoney - cost;
}
bool isDrinkSoldOut(Beverage drink) {
    return (drink.numberOfDrinks <= 0);
}
bool hasEnoughMoney(Beverage drink, double userMoney) {
    return (drink.drinkCost <= userMoney);
}
int getUserMoney() {
    int userMoney;
    do {
        promptUserForMoney();
        std::cin >> userMoney;
        std::cin.clear();
        std::fflush(stdin);
        std::cin.ignore(10000,'\n');
    } while(userMoney < 0);
    return userMoney;
}
void promptUserForMoney() {
    std::cout << "Enter any amount of money greater than zero: " << std::endl;
}
void initialDrinkMachinePrompt(Beverage drinks[], int numOfDrinks) {
    std::cout << "Here is the list of out drinks: " << std::endl;
    displayDrinks(drinks, numOfDrinks);
}
void promptDrinkMachineMenu(int numOfDrinks) {
    std::cout << "Please choose a number from " << INDEX_TO_NON_ZERO_LIST
    << " to " << numOfDrinks << " (or '"<< QUIT_CHARACTER <<"' to close the machine) : ";
}
char getDrinkMachineMenuResponse(int numOfDrinks) {
    char response = '0';
    do {
        promptDrinkMachineMenu(numOfDrinks);
        std::cin >> response;
        response = std::tolower(response);
        std::cin.clear();
        std::fflush(stdin);
        std::cin.ignore(10000,'\n');
    } while(!(isValidDrink(response,numOfDrinks)|| response == QUIT_CHARACTER));
    return response;
}
bool isValidDrink(char response, int numOfDrinks) {
    int responseInt = response - '0';
    return (INDEX_TO_NON_ZERO_LIST<=responseInt && responseInt <= numOfDrinks);
}
void displayDrinks(Beverage drinks[], int numOfDrinks) {
    int maxDrinkNameStrLength = findMaxDrinkNameStrLength(drinks, numOfDrinks);
    int maxNumOfDrinksStrLength = findMaxNumOfDrinksStrLength(drinks, numOfDrinks);
    for(int i = 0; i < numOfDrinks; ++i) {
        displayListNumberedItem(i+INDEX_TO_NON_ZERO_LIST);
        displayDrink(drinks[i],maxDrinkNameStrLength, DRINK_COST_MAX_STR_LENGTH, maxNumOfDrinksStrLength);

    }
}
void displayListNumberedItem(int itemNumber) {
    std::cout << "\t " << itemNumber << ". ";
}

int findMaxDrinkNameStrLength(Beverage drinks[], int numOfDrinks) {
    int maxLength = static_cast<int>(drinks[0].drinkName.length());
    for(int i = 1; i < numOfDrinks; ++i)
        if(drinks[i].drinkName.length() > maxLength)
            maxLength = static_cast<int>(drinks[i].drinkName.length());
    return maxLength + ONE_SPACE_PADDING;
}
int findMaxNumOfDrinksStrLength(Beverage drinks[], int numOfDrinks) {
    int maxLength = static_cast<int>(std::to_string(drinks[0].numberOfDrinks).length());
    for(int i = 1; i < numOfDrinks; ++i)
        if(std::to_string(drinks[i].numberOfDrinks).length() > maxLength)
            maxLength = static_cast<int>(std::to_string(drinks[i].numberOfDrinks).length());
    return maxLength + ONE_SPACE_PADDING;
}
void displayDrink(Beverage drink, int nameStrLength, int costStrLength, int numOfDrinksStrLength) {
    std::cout << std::setw(nameStrLength) << std::left << drink.drinkName
    << std::setw(costStrLength) << std::setprecision(DRINK_COST_PRECISION) << std::fixed << drink.drinkCost
    << std::setw(3) << drink.numberOfDrinks
    << std::endl;
}

This solution works and in this assignment I tried to make my code as self-documenting as possible and I'm wondering if this would be too verbose for industry? Does the verboseness add clarity or does it remove from it? How can I improve the code to be more readable?

There aren't any real restrictions on this assignment but it was assigned before classes and templates so my professor doesn't expect me to be using them. Is this assignment verbose because I'm not using classes and templates?

\$\endgroup\$
  • 1
    \$\begingroup\$ Any restrictions on what you're allowed to use? No containers, C++11/14? \$\endgroup\$ – Johnbot Aug 12 '16 at 6:48
  • \$\begingroup\$ Not really, just implied knowledge restrictions. \$\endgroup\$ – Harrison Tran Aug 12 '16 at 6:54
  • 1
    \$\begingroup\$ Classes would definitely make this more straight-forward, there's duplication here and there but it doesn't look too bad. \$\endgroup\$ – Mast Aug 12 '16 at 7:37
2
\$\begingroup\$

Disclaimer: I'm primarily a C# developer

No using namespace std; get's a huge thumbs up!

Your code looks like C written in C++.

Stop using raw arrays and start using container types such as std::array, std::vector and so on.

Use references over pointers when possible. Make them constant references when you don't need to modify it.

void displayDrinks(Beverage drinks[], int numOfDrinks);

becomes

void displayDrinks(std::vector<Beverage> const& drinks);

and you can use drinks.size() instead of passing along an extra parameter.

If you need to keep a pointer to an object you own use std::unique_ptr (std::make_unique) and std::shared_ptr (std::make_shared) if ownership is shared. Plain pointers is only for when you don't own the object and someone else is responsible for deallocating it. Now you won't ever have to call new and delete.

Use size_t for iterators and indexers and int32_t, uint32_t, int64_t, uint64_t when you care about sizes and signedness. Plain int is not guaranteed to be 32bit. Remember signed overflow is undefined.

Always use std::string for string manipulation and comparisons. char and char* can be hard to handle correctly.

Start encapsulating logic in classes when you learn about them. Use both header files and implementation files.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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