HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Right triangle side length calculation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
calculationsidelengthtriangleright

Problem

My code compiles and executes. I would like to hear any comments or suggestions pertaining to my use of C++.

#include 
#include 

bool error_check(){
    if(!std::cin){
        std::cin.clear();
        std::cin.ignore(256,'\n');
        return true;        
    }
    else 
        return false;
};

int main(){

//This line outputs directions for the user to follow.
std::cout > sidea;
    //This condtional branch handles bad user input which is in this case anything but real numbers.
    if(error_check()){
        std::cout > sideb;
    //This condtional branch handles bad user input which is in this case anything but real numbers.
    if(error_check()){
        std::cout > sidec; 
    //This condtional branch handles bad user input which is in this case anything but real numbers.
    if(error_check()){
        std::cout << "\nYou must enter a real number.\n" << std::endl;      
        continue;
    }

    //This conditional branch calculates and outputs the unknown side length.
    if(sidea == 0){
        sidea = sqrt(pow(sidec,2) - pow(sideb,2));
        std::cout << "side a= " << sidea << "\n" << std::endl;
    }
    else if(sideb == 0){
        sideb = sqrt(pow(sidec,2) - pow(sidea,2));
        std::cout << "side b= " << sideb << "\n" << std::endl;
    }
    else if(sidec == 0){
        sidec = sqrt(pow(sidea,2) + pow(sideb,2));
        std::cout << "side c= " << sidec << "\n" << std::endl;
    }
    else{
        std::cout << "\nYou must enter zero for an unknown side length.\n" << std::endl;
        continue;
    }

    //This conditional branch checks to see if the user wants to exit. 
    if(sidea == 0 and sideb == 0 and sidec == 0)
        break;
    else
        continue;

}
return 0;
};

Solution

Holy comments Batman!

You have too many useless comments which makes your code harder to follow.


Comments should explain why something is done, not what is done.

Good code makes it obvious what is done, if you feel that you need to comment what is being done then your code can be improved instead.

I'm going to link an answer to another question that explains commenting in a very good way (IMHO).

Prefer && to and

This code if(sidea == 0 and sideb == 0 and sidec == 0) had me do a double take, the and totally threw me off as I'm very used to see && used instead.

Keep it simple

One simplification you can make is to change bool error_check() into float read_real(). You should also extract your exit critera into a function and use it as a loop condition instead of breaking out of an infinite loop. This makes it much more clearer and removes the need for the comment. You can also extract computations from the conditional block when calculating the unknown side.

Like this:

float read_real(){
    float ans;
    std::cin >> ans;
    if(!std::cin){
        std::cin.clear();
        std::cin.ignore(256,'\n');
        throw std::invalid_argument("Please enter a real number");
    }
    return ans;
};

bool should_exit(float a, float b, float c){
    return a == 0 && b == 0 && c == 0;
}

...

float sidea = 1.0f;
float sideb = 1.0f;
float sidec = 1.0f;
while(!should_exit(sidea, sideb, sidec)){
    try{
        std::cout << "side a: "; 
        sidea = read_real();

        std::cout << "side b: ";
        sideb = read_real();

        std::cout << "side c: ";
        sidec = read_real();

        // The unknown side must be 0, so this will work for all cases.
        float unknown = sqrt(abs(sidea*sidea + sideb*sideb - sidec*sidec));
        if(sidea == 0){
            std::cout << "side a= " << unknown << "\n" << std::endl;
        }
        else if(sideb == 0){
            std::cout << "side b= " << unknown << "\n" << std::endl;
        }
        else if(sidec == 0){
            std::cout << "side c= " << unknown << "\n" << std::endl;
        }
        else{
            throw std::invalid_argument("You must enter zero for an unknown side length.");
        }
    } catch (std::invalid_argument e){
        std::cout<<"\n Error: "<<e.what()<<std::endl;
    }
}

Code Snippets

float read_real(){
    float ans;
    std::cin >> ans;
    if(!std::cin){
        std::cin.clear();
        std::cin.ignore(256,'\n');
        throw std::invalid_argument("Please enter a real number");
    }
    return ans;
};

bool should_exit(float a, float b, float c){
    return a == 0 && b == 0 && c == 0;
}

...

float sidea = 1.0f;
float sideb = 1.0f;
float sidec = 1.0f;
while(!should_exit(sidea, sideb, sidec)){
    try{
        std::cout << "side a: "; 
        sidea = read_real();

        std::cout << "side b: ";
        sideb = read_real();

        std::cout << "side c: ";
        sidec = read_real();

        // The unknown side must be 0, so this will work for all cases.
        float unknown = sqrt(abs(sidea*sidea + sideb*sideb - sidec*sidec));
        if(sidea == 0){
            std::cout << "side a= " << unknown << "\n" << std::endl;
        }
        else if(sideb == 0){
            std::cout << "side b= " << unknown << "\n" << std::endl;
        }
        else if(sidec == 0){
            std::cout << "side c= " << unknown << "\n" << std::endl;
        }
        else{
            throw std::invalid_argument("You must enter zero for an unknown side length.");
        }
    } catch (std::invalid_argument e){
        std::cout<<"\n Error: "<<e.what()<<std::endl;
    }
}

Context

StackExchange Code Review Q#90096, answer score: 3

Revisions (0)

No revisions yet.