Code rating

Dear fellow PH forum guests,

On the internet I found the code below. The code does what it is supposed to do: solving quadratic equations. My question is: if you were to rate this code on a scale from 1 (terrible, couldn't be worse) to 10 (expert, couldn't be better), knowing that the programmer makes a living out of programming. All indentation and comments is his/hers.

Thanks you for you time, Bilderbikkel

[code]
#include
#include
#include
using namespace std;

void one(){

float a = 0.0; //here we declare the variables and use float because we
float b = 0.0; //are dealing with square roots
float c = 0.0;
float x1 = 0.0;
float x2 = 0.0;
float x3 = 0.0;
float x4 = 0.0;

//this section gets user input and displays message
cout << "Enter the coefficients a , b , c for equation in the form ax^ + bx + c = 0:
";
cout << "Enter value for a:
";
cin >> a;
cout << "Enter value for b:
";
cin >> b;
cout << "Enter value for c:
";
cin >> c;

//are all the coefficients 0? if so both roots are 0
if(a == 0 && b == 0 && c == 0){
x1 = 0;
x2 = 0;
cout << "The roots are:" "
"
<< "x1 = " << x1 << " , " << "x2 = " << x2 << "
";
}

//is c the only non-zero number? if so tell the user
if(a == 0 && b == 0 && c != 0){
c = c;
cout << "There are no roots" "
"
<< "c = " << c << "
";
}
//is a zero? if so solve the resulting linear equasion and notify user
if(a == 0 && b != 0 && c !=0){
cout << "The values entered do not make a quadratic expression" "
"
<< "x = " << -c/b << "
";
}

//if b is zero and c is zero tell user
if(a == 0 && b != 0 && c == 0){
x1 = 0;
x2 = 0;
cout << "The roots are:" "
"
<< "x1 = " << x1 << " , " << "x2 = " << x2 << "
";
}
//if b and c are equal to zero then ax^=0 and since a cannot be zero without x being
// zero also let user know
if(a != 0 && b == 0 && c == 0){
x1 = 0;
x2 = 0;
cout << "The values entered result in ax^= 0; so both roots are 0" "
"
<< "x1 = " << x1 << " , " << "x2 = " << x2 << "
";
}
//factor out x from ax^+bx=0 and either x = 0 or ax + b =0
//then solve the linear equation
if(a != 0 && b != 0 && c == 0){
x1 = 0;
x2 = -b/a;
cout << "The roots are:" "
"
<< "x1 = " << x1 << " , " << "x2 = " << x2 << "
";
}

//now we get to use the square root function and let the user
//know they have some imaginary numbers to deal with
if(a < 0 && b == 0 && c < 0){

x1 = -b/(2*a);
x4 = (b*b)-(4*a*c);
x4 = -x4;
x2 = sqrt(x4)/(2*a);
x3 = -sqrt(x4)/(2*a);


cout << "The roots are not real numbers:" "
"

<< "x1 =" << x1 << " + " << x2 << " * i " << "
"
<< "x2 =" << x1 << " + " << x3 << " * i " << "
";

}

if(a > 0 && b == 0 && c > 0){

x1 = -b/(2*a);
x4 = (b*b)-(4*a*c);
x4 = -x4;
x2 = sqrt(x4)/(2*a);
x3 = -sqrt(x4)/(2*a);


cout << "The roots are not real numbers:" "
"

<< "x1 =" << x1 << " + " << x2 << " * i " << "
"
<< "x2 =" << x1 << " + " << x3 << " * i " << "
";



}
//now a and c are opposite signs so the answer will be real

if(a > 0 && b == 0 && c < 0){

x1 = (-b + (sqrt(pow(b,2)-(4*a*c))))/(2*a);
x2 = (-b - (sqrt(pow(b,2)-(4*a*c))))/(2*a);

cout << "The roots are:" "
"
<< "x1 = "<< x1 << " , " << "x2 = "<< x2 << "
";
}
if(a < 0 && b == 0 && c > 0){

x1 = (-b + (sqrt(pow(b,2)-(4*a*c))))/(2*a);
x2 = (-b - (sqrt(pow(b,2)-(4*a*c))))/(2*a);

cout << "The roots are:" "
"
<< "x1 = "<< x1 << " , " << "x2 = "<< x2 << "
";
}


//ok now if we end up not having to take the square root of a neg
// do the math
if(a != 0 && b != 0 && c != 0 && (4*a*c) <= pow(b,2)){


x1 = (-b + (sqrt(pow(b,2)-(4*a*c))))/(2*a);
x2 = (-b - (sqrt(pow(b,2)-(4*a*c))))/(2*a);

cout << "The roots are:" "
"
<< "x1 = "<< x1 << " , " << "x2 = " << x2 << "
";
}

//here we have to deal with non x intercepts ie: sqrt(-1)
// alter the formula slightly to give correct output and
// let the user know
if(a != 0 && b != 0 && c != 0 && (4*a*c)> pow(b,2)){


x1 = -b/(2*a);
x4 = (b*b)-(4*a*c);
x4 = -x4;
x2 = sqrt(x4)/(2*a);
x3 = -sqrt(x4)/(2*a);


cout << "The roots are not real numbers" "
"

<< "x1 =" << x1 << " + " << x2 << " * i " << "
"
<< "x2 =" << x1 << " + " << x3 << " * i " << "
";




}
char g;
cout << "Solve another equation? (y for yes or n for no)
";
cin >> g;
if(g !='y'){
exit(0);
}

return;

}

//keep output from vanishing before we can read it.
void two(){
char g ;
cout << "Press c and then Enter to continue...." "
";
cout << "Press s and then enter to stop....." "
";
cin >> g;
if (g == 'c')
for(int i=0; i<100; i++)// who wants to run this more that 99 times? if you are using this program for
//any thing other than some quick calculations (for example as part of another program)
// you may want to comment this part out. The for loop keeps the program from
//running away in an infinate loop, but may not be usable in another program.
//If 99 times is not enough, change the value in the for loop, but be careful
//with no limits the for loop can run the program in the background by accident



one();

}

int main(){
one();
two();
return 0;
}

[/code]
bilderbikkel

Comments

  • By taking a brief look at it, I'd say it is around 2.


    Main issues are:

    - Poor indention. It isn't readable.

    - Lack of modular programming. It should have functions or a class for the calculations, and those functions shouldn't contain user i/o.

    - Obfuscation. Lines like
    x1 = (-b + (sqrt(pow(b,2)-(4*a*c))))/(2*a);
    aren't readable.

    - The code doesn't handle the line feed character from stdin.

    - Implicit typecasts between int and float. This is a potential bug, as one doesn't know if "int" is signed or unsigned when dealing with portable code.



    Minor issues:

    - Implicit typecasts between float and double. Creates unnecessary execution time and program size.

    - No function prototypes.

    - math.h instead of

    - The code is using the questionable exit() function.
Sign In or Register to comment.

Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Categories

In this Discussion