patterncppMinor
Right triangle side length calculation
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
This code
Keep it simple
One simplification you can make is to change
Like this:
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 andThis 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.