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.

Please evaluate this code and tell me if it is standard C++ coding:

#include <iostream>
#include "DivisionByZero.h"
using namespace std;

double checkQuotient(int numerator, int denominator){
       if(denominator == 0)
            throw DivisionByZero();

       return static_cast<double>(numerator/denominator);
}


int main(){
    int num1, num2;
    double ans;

    cout << "Enter 2 numbers: \t";
    while(cin >> num1 >> num2){

    try{
        ans = checkQuotient(num1, num2); 
        cout << "Your answer is: " << ans << endl;   
    }
    catch(DivisionByZero &divError){
           cout << "Exception occurred: " << divError.what() << endl;      
     }
     catch(string x){
           cout << x;             
     }
     catch(int z){
               cout << z;
     }

     cout << "Enter two numbers " << endl;
     }



    system("pause");
    return 0;    
}

I just started trying exception handling and just need someone to verify if this is the normal standard for C++ programmers worldwide.

share

migration rejected from stackoverflow.com Mar 31 at 16:54

This question came from our site for professional and enthusiast programmers. Votes, comments, and answers are locked due to the question being closed here, but it may be eligible for editing and reopening on the site where it originated.

closed as off-topic by 200_success Mar 31 at 16:54

  • This question does not appear to be a code review request within the scope defined in the help center.
If this question can be reworded to fit the rules in the help center, please edit the question.

    
This question was heavily edited by the author such that it is now completely different in nature from the original text. Closing this question to prevent further confusion. –  200_success Mar 31 at 16:54

2 Answers 2

Stop doing this its a bad habit and breaking it can be hard if you get used to it:
Why is “using namespace std;” considered bad practice?

using namespace std;

This is not doing what you think:

       return static_cast<double>(numerator/denominator);

Its doing integer division. Only after that is it being converted to a double for the result. To get float division at least one of the parameters must be a double. You can use static_cast<double> for this but it is more common to multiply by 1.0 (as the literal 1.0 is double).

 return (numerator*1.0)/denominator;

 // or 

 return static_cast<double>(numerator)/denominator;

Declare variables as close to the usage point as possible.
This way it is easy to see the type without scrolling to the top of the function. Also when you start using objects the constructor is run at the point in the code that it is declared and this can become important.

double ans;

// Move down.
double ans = checkQuotient(num1, num2);

User input is usually line based. Though your code will work fine you should keep this in mind for future work as mixing line based std::getline() and stream based input operator>>() can lead to odd issues for the inexperienced.

while(cin >> num1 >> num2){

You may want to think about what happens if the user inputs:

12 13 14<enter>

You print the result. Then ask for two integers. But the next user enters:

7 1<enter>

They will be surprised that the answer is 2 not 7.

You may want to tell the user this is an exception in these two cases.

 catch(string x){
       cout << x;             
 }
 catch(int z){
           cout << z;
 }

Try and use platform agnostic pause.

system("pause");

Don't return anything from main.

return 0;    

This indicates that the application can not fail. If you return something it indicates to me there are other instances where you are returning a non zero value and thus there are failure exits.

share
  • Keep your indentation consistent. It's common to have four spaces of indentation where indentation is needed. Lack of consistency can indicate a neglect of attention to clean code.

  • Prefer to include user-defined headers (#include "/*...*/") above library headers (#include </*...*/>). This will prevent the user-defined headers from having to use the library ones, which may lead to broken code.

  • As mentioned by @Loki Astari, prefer not to use system("PAUSE") as it's not portable (Windows only). There are similar alternatives that are portable, such as std::cin.get(). The only difference is that this function will wait for an input instead of just terminating after pressing a key.

share