Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The problem is to input, divide 2 ints and display the result.
Please, review my program. It worth noting, I've intentionally used goto, not while.

int main(){
    int a, b;

ab_input:
    cout << "Enter 2 numbers: ";
    cin >> a >> b;
    try {
        if (!cin) throw std::runtime_error("Bad input");
        if (b == 0) throw std::runtime_error("Dividing by zero");
    } 
    catch (std::runtime_error er) {
        cin.sync();
        cin.clear();
        std::cerr << er.what() << endl;
        cout << "Try again? y/n: ";
        char c; 
        cin >> c;
        if (cin && c == 'y') goto ab_input;
        else return -1;
    }
    cout << "a/b = " << a / b << endl;

    return 0;
}
share|improve this question
12  
Could you explain why you've intentionally used goto? –  Jamal Mar 26 at 16:31
10  
Hope the raptors get you ... ;-) .... –  rolfl Mar 26 at 16:33
1  
(LET THE HOLYWAR BEGINS!!!11) I believe it's more apropriate and more readable here, than while. Or goto_is_bad is a dogma? Anyway, may be there are some another notes? –  user2198121 Mar 26 at 16:38
2  
"Or goto_is_bad is a dogma?" -- It's popularly Considered harmful. –  ChrisW Mar 26 at 16:57
3  
@user2198121: Its not a holy war. That implies there are two large factions that disagree. I think you will find there is only one large faction (that war was won by our side decades ago). But yes there are legitimate uses of goto (they do exist and I have used goto once). But here it is not legitimate and does make the code harder to read. –  Loki Astari Mar 26 at 18:14

1 Answer 1

up vote 12 down vote accepted

Some alternatives to goto:

int main(){
    int a, b;

    for (;;) { // until success
        cout << "Enter 2 numbers: ";
        cin >> a >> b;
        try {
            if (!cin) throw std::runtime_error("Bad input");
            if (b == 0) throw std::runtime_error("Dividing by zero");
            break; // success!
        } 
        catch (const std::runtime_error& ex) {
            cin.sync();
            cin.clear();
            std::cerr << ex.what() << endl;
            cout << "Try again? y/n: ";
            char c; 
            cin >> c;
            if (cin && c == 'y') continue; // try again!
            else return -1;
        }
    }
    cout << "a/b = " << a / b << endl;

    return 0;
}

or:

static bool tryAgain(const char* errorMessage){
    cin.sync();
    cin.clear();
    std::cerr << errorMessage << endl;
    cout << "Try again? y/n: ";
    char c; 
    cin >> c;
    return (cin && c == 'y'); // try again!
}

int main(){
    int a, b;

    const char* errorMessage;
    do {
        cout << "Enter 2 numbers: ";
        cin >> a >> b;
        // set errorMessage.
        if (!cin) errorMessage = "Bad input";
        else if ((b == 0)) errorMessage = "Dividing by zero";
        else errorMessage = 0;
        if (errorMessage && !tryAgain(errorMessage)) return -1; // fail!
    } while (errorMessage);
    cout << "a/b = " << a / b << endl;

    return 0;
}

Or:

int main(){
    int a, b;

    for (;;) { // Until explicit break on success.
        cout << "Enter 2 numbers: ";
        cin >> a >> b;
        // set errorMessage.
        const char* errorMessage;
        if (!cin) errorMessage = "Bad input";
        else if ((b == 0)) errorMessage = "Dividing by zero";
        else break; // success!
        if (!tryAgain(errorMessage)) return -1; // fail!
    }

    cout << "a/b = " << a / b << endl;

    return 0;
}

Also you might like to change the last statement to:

cout << "a/b = " << (double)a / b << endl;

I would also mention that returning -1 is normally a bad idea. The shell interprets negative numbers in weird ways

All of the Exit status - References suggest 0 for success and non-zero (but positive) for failure:

  • Some (Unix and DOS) references suggest that the range of valid codes is 0..255
  • Another (HP OpenVMS) says that bits 29..31 are reserved and must be zero
  • The Windows System Error Codes range up to 15999 (I guess you might want to return something like ERROR_INVALID_PARAMETER).
share|improve this answer
4  
I would also mention that returning -1 is normally a bad idea. The shell interprets negative numbers in weird ways –  FDinoff Mar 26 at 18:50

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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