6
void GasPump::dispense()
{

        bool cont = true;
        char stop;

    do{
        cout << "Press any key, or enter to dispense.\n"
             << "Or press 0 to stop: \n";
        cin.get(stop);

        gasDispensed = gasDispensed + gasDispensedPerCycle;
        charges = costPerGallon*gasDispensed;
        displayGasNCharges();

        if(stop == 0)
            cont = false;

    } while(cont);

}

Doing an assignment, this is my first program to write with objects so bear with me. I just can't get the output of this code to turn out right. I need a way to get out of the loop, and what I'm using just isn't working. Any suggestions, hints or tips?

Machavity
  • 30,841
  • 27
  • 92
  • 100

2 Answers2

5

Try comparing stop to the zero char.

stop == '0'

Also you can simplify your code by doing this.

void GasPump::dispense()
{
    char stop;

    do {
        cout << "Press any key, or enter to dispense.\n"
             << "Or press 0 to stop: \n";
        cin.get(stop);

        gasDispensed = gasDispensed + gasDispensedPerCycle;
        charges = costPerGallon*gasDispensed;
        displayGasNCharges();
    } while (stop != '0');
}
ChaosPandion
  • 77,506
  • 18
  • 119
  • 157
  • Ooh! I didn't realize when using a decimal number as a char you had to put the quotes around it, good to know, thanks a lot bud! –  Feb 27 '10 at 04:15
  • 2
    @blakejc70 Since stop is a char and with C++ method overloading your call to cin.get is routed through to an input handling logic that will return a character code. So stop == '0' == 48 ( going by assumption of ASCII character set ). – David Feb 27 '10 at 05:32
3

In this scenario, you pump gas one extra time after the user hits '0'. Assuming that this is not desired, you have what is known as an "off-by-one error." You can fix this (and eliminate the temporary variable) by rearranging your function as follows:

void GasPump::dispense()
{
    while (true) {
        cout << "Press any key, or enter to dispense.\n"
             << "Or press 0 to stop: \n";

        if (cin.get() == '0')
            break;

        gasDispensed = gasDispensed + gasDispensedPerCycle;
        charges = costPerGallon*gasDispensed;
        displayGasNCharges();
    }
}

To avoid using a break statement, you can use the following construction:

bool GasPump::shouldDispenseGas()
{
    cout << "Press any key, or enter to dispense.\n"
         << "Or press 0 to stop: \n";
    return (cin.get() != '0');
}

void GasPump::dispense()
{
    while (shouldDispenseGas()) {
        gasDispensed = gasDispensed + gasDispensedPerCycle;
        charges = costPerGallon*gasDispensed;
        displayGasNCharges();
    }
}

EDIT (2011 September 27): @TonyK Just because a language provides a feature doesn't mean that one should use it. The goto statement is a classic example of this.

Granted, with such a simple loop, there's really no difference between using a function and the break. Both are clear. However, when extra features get added a month (or years) later, along with extra conditions for breaking out of the loop, it's very easy to find multiply-nested if statements with complex logic inside a loop that's so large, you have a hard time finding its beginning, much less the exit points. One of the ways to fight this type of code bloat is to write short, simple, and focused functions that are well-named. If you do this, the code documents itself. Compare

while (true)

versus

while (shouldDispenseGas())

Similarly, compare this to the STL for_each algorithm. Sure, std::for_each(v.begin(), v.end(), &foo); is a little shorter than for (int i = 0; i < v.size(); ++i) { ...body of foo()... }. But the real advantage is that it's easier to see what the intent is. In the for_each you immediately see that you will be doing something once, and only once, to each element. In the for loop, you have no idea. The loop counter i may be changed in the loop. A break may be hidden inside as well. By shirking this break statement and embedding the logic in shouldDispenseGas, you immediately understand the conditions under which the loop will continue, and end.

JKD
  • 623
  • 5
  • 8
  • Hey thanks for your help. I was wondering how to stop that, and I thought about using a break, but my textbook is telling me it's not good practice, although my professor never said that. What's your thoughts on this? –  Mar 02 '10 at 05:29
  • In general, I try to put the "break" logic into the comparison being down by the while or for statement. However, sometimes it's just easier to use a break. In this case, where it's incredibly clear what's going on, I don't think there's a problem with using break. That being said, you certainly can avoid using it while maintaining readability at the cost of writing a separate function (as shown in the edited response). – JKD Mar 04 '10 at 22:49
  • +1 The second version with the extra function gives me the warm fuzzies. :-) – Jeffrey L Whitledge Mar 09 '10 at 14:53
  • The second version is the worst kind of prescriptivism. The only reason for the extra function is to avoid the `break` statement. But that is precisely what `break` statements are for! Why not use them? – TonyK Sep 02 '11 at 19:11