Simple Quadratic Equation Solver - C++ - What Do You Think?

[f ben isaac]

Please tell me what do you think of the written code below, is it easy to follow? Does the code tells you what it does, or you have struggled realizing whats going on? and so on. Feel free to give suggestions, tips, advices and criticize! This will help me and help other members too...

Note: i'm trying to know how other developers see my code & trying to see where i go wrong. Thus, the best way to evaluate myself is to post my source code and see what do you think about it.

/**program description:
 *
 * this program is a simple quadratic equation 
 * solver. I tried to make the code talks about
 * itself by giving good descriptive names and
 * reducing unneeded comments
 *
 * simple exercise for you: validate user input
 *
 * Author: X-Shab
 *
 */

#include <iostream>
#include <cmath>
using namespace std;

void quadraticFormula(double a, double b, double c, double *totalResults)
{
	double numeratorSqrtResult = sqrt( (b*b) - (4*a*c) );
	double denominatorResult = 2 * a;
	double addNegativeTo_b = 0 - b;

	if (denominatorResult == 0)
	{
		cout<<"Denominator is zero\n";
		exit(0);
	}
	
	totalResults[0] = (addNegativeTo_b + numeratorSqrtResult)/denominatorResult;
	totalResults[1] = (addNegativeTo_b - numeratorSqrtResult)/denominatorResult;
}

int main(void)
{
	double a, b, c;
	double x[2];       //store results of x

	cout<<"Please Enter a, b, and c to get Results of x:\n";

	cout<<"a: ";
	cin>>a;

	cout<<"b: ";
	cin>>b;

	cout<<"c: ";
	cin>>c;

	quadraticFormula(a, b ,c, &x[0]);

	cout<<"\nYour Results are:"<<endl;

	for(int i = 0; i < 2; i++)
	{
		cout<<"x = "<< x[i] <<endl;
	}

	system("pause");
	return 0;
}

[Bazzy]

you can have it with less typing:

bool quadraticFormula(double a, double b, double c, double *totalResults)
{
    double D = sqrt( (b*b) - (4*a*c) );//shorter name

    if (!a)// a==0 => 2*a==0
    {
        cout<<"Denominator is zero\n";
        return false; // function failed -you can also use exceptions-
    }
	
    totalResults[0] = ( -b + D)/(2*a);
    totalResults[1] = ( -b - D)/(2*a);

    return true;//function succeded
}

int main(void)
{
    double a, b, c;
    double x[2];

    cout<<"Please Enter a, b, and c to get Results of x:\n";

    cout<<"a: ";
    cin>>a;

    cout<<"b: ";
    cin>>b;

    cout<<"c: ";
    cin>>c;

    if ( quadraticFormula(a, b ,c, x) )
    {
        cout<<"\nYour Results are:"<<endl;
        cout << x[0] << endl << x[1] << endl; //you don't really need a loop here
    }
    system("pause");//better not using this, see http://www.cplusplus.com/forum/articles/7312/
    return 0;
}

[jsmith]

From a readability standpoint it is well structured, concise, and easy to understand.

[f ben isaac]

Thanks! This helped

[jsmith]

From a usability standpoint, it is possible for there to be zero, one, or two solutions. The function as written does not allow the user to know how many solutions there are. Moreover, in the case of zero solutions, the function likely tries to take the square root of a negative number which will probably crash (can't try it at the moment).

Also, IMNSHO (though others will differ), I'd rather see the function take two reference parameters instead of a pointer because pointers are prone to programming error (pointer to unallocated memory, memory block that is too small, NULL pointer, etc), whereas references are much easier to get right. Another approach could be to return a struct containing the roots and the number of valid roots.

From a technical standpoint, the method you are using to compute the roots is prone to signifcant error if b^2 ~= 4ac due to catastrophic cancellation (research What Every Computer Scientist Should Know About Floating Point Arithmetic). [Catastrophic cancellation essentially means that the result will be accurate to zero decimal places.]

[f ben isaac]

jsmith your reply is helpful. It will be taken into consideration when in future coding.

Here is a quick fixings i did before you last response: (i just posted it so people other can benefit from it - hopefully)

/**program description:
 *
 * this program is a simple quadratic equation 
 * solver. I tried to make the code talks about
 * itself by giving good descriptive names and
 * reducing unneeded comments
 *
 * simple exercise for you: validate user's input
 *
 * Author: X-Shab
 *
 */

#include <iostream>
#include <cmath>
using namespace std;

bool quadraticFormula(double a, double b, double c, double totalResults[])
{
	if( ((b*b) - (4*a*c)) < 0 )
	{
		cout<< "\nSquare root can not be performed with a negative number" <<endl;
		return false;
	}
	double numeratorSqrtResult = sqrt( (b*b) - (4*a*c) );
	double denominatorResult = 2 * a;
	double addNegativeTo_b = 0 - b;

	if (denominatorResult == 0)
	{
		cout<<"Denominator is zero\n";
		return false;
	}
	
	totalResults[0] = (addNegativeTo_b + numeratorSqrtResult)/denominatorResult;
	totalResults[1] = (addNegativeTo_b - numeratorSqrtResult)/denominatorResult;

	return true;
}

int main()
{
	double a, b, c;
	double x[2];       //store results of x

	cout<<"Please Enter a, b, and c to get Results of x:\n";

	cout<<"a: ";
	cin>>a;

	cout<<"b: ";
	cin>>b;

	cout<<"c: ";
	cin>>c;

	if (quadraticFormula(a, b ,c, x))
	{
		cout<<"\nYour Results are:"<<endl;
		cout<<"x = "<< x[0] <<endl;
		cout<<"x = "<< x[1] <<endl;
	}

	//try to avoid using system("pause"): http://www.gidnetwork.com/b-61.html
	cin.get();
	return 0;
}