7
\$\begingroup\$

I likes my use of functions so I'm mostly looking for little things but if you see something tell me :)

#include <iostream>


using namespace std;


void addQuarter(double &num); void addDime(double &num); void 
addNikle(double &num); void addPennie(double &num);
bool quarterValid(double num, const double finalNum); bool dimeValid(double num, const double finalNum); bool nikleValid(double num, const double finalNum); bool pennieValid(double num, const double finalNum);

class change {
public:
    change() = default;
    change(double a) { setChange(a); }
    void getChange();
    void setChange(double change) { totalAmmount = change; }
    int getQuaters() const { return ammountQuarters; }
    int getDimes() const { return ammountDimes; }
    int getNikles() const { return ammountNikles; }
    int getPennies() const { return ammountPennies; }
private:
    double totalAmmount; double ammountSoFar = 0;
    int ammountQuarters = 0, ammountDimes = 0, ammountNikles = 0, ammountPennies = 0;
};

void change::getChange() {
    while (quarterValid(ammountSoFar, totalAmmount)) {
         addQuarter(ammountSoFar);
         ++ammountQuarters;
    }
    while (dimeValid(ammountSoFar, totalAmmount)) {
         addDime(ammountSoFar);
         ++ammountDimes;
    }
    while (nikleValid(ammountSoFar, totalAmmount)) {
        addNikle(ammountSoFar);
        ++ammountNikles;
    }
    while (pennieValid(ammountSoFar, totalAmmount)) {
        addPennie(ammountSoFar);
        ++ammountPennies;
    }
}

void addQuarter(double &num) {
    num += .25;

void addDime(double &num) {
    num += .10;
}
void addNikle(double &num) {
    num += .05;
}
void addPennie(double &num) {
    num += .01;
}

bool quarterValid(double num, const double finalNum) {
    if ((num + .25) <= finalNum)
        return true;
    return false;
}

bool dimeValid(double num, const double finalNum) {
    if ((num + .10) <= finalNum)
        return true;
    return false;
}

bool nikleValid(double num, const double finalNum) {
    if ((num + .05) <= finalNum)
        return true;
    return false;
}

bool pennieValid(double num, const double finalNum) {
    if ((num + .01) <= finalNum)
        return true;
    return false;
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Don't use double for monetary values. Use integer types just make it multiples of the smallest denomination (ie in US use cents). \$\endgroup\$ – Martin York Dec 31 '15 at 16:14
11
\$\begingroup\$

Class vs function

The problem you're solving is basically: given $X, make change. That sounds like a function to me:

??? makeChange(double amount);

As in - you're returning something that tells the user how many quarters, pennies, etc, are needed. But if you make change a class like this, it's solving two problems - both the making change problem and the determining the solution. That's mixing responsibilities. At least, you should make makeChange static - certainly there's no reason for change to hold onto totalAmmount, nor for it to be default-constructible.

But really, what makes the most sense is to have a normal POD:

struct change {
    int quarters;
    int dimes;
    int nickels;
    int pennies;
};

And just return that.

using namespace std;

Don't do it. Especially in a header. Especially when you're not even using anything from std. You don't need the <iostream> import either.

Use of functions

This is too many functions. First of all, if you're going to declare your functions up front and define them later, one function per line. But secondly, do you really need separate functions for each coin? They all do the same thing - the only difference is the amount. So just make that another argument:

int makeChangeWith(double& amount, double coin)
{
    int num = 0;
    while (amount >= coin) {
        amount -= coin;
        ++num;
    }
    return num;
}

And then your getChange() becomes:

change makeChange(double amount) {
    change result;

    result.quarters = makeChangeWith(amount, 0.25);
    result.dimes = makeChangeWith(amount, 0.10);
    result.nickels = makeChangeWith(amount, 0.05);
    result.pennies = makeChangeWith(amount, 0.01);

    return result;
}

Moreover, once we do it this way, we don't have to subtract one at a time. We can just divide. If we have $5.01, I don't need to subtract off 20 quarters, I can see that (int)(5.01 / .25) is 20 and go from there.

Floating point math

Floating point math is annoying and imprecise. It'd be better to avoid it entirely. Convert your values to an integral number of cents up front:

int cents = amount * 100;

And then decrease/divide by 25, 10, 5, and 1 instead.

Spelling

The coins are a nickel and a penny, not nikle and pennie.

\$\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.