Review my quadratic formula program please.

[mLindroth]

I'm new to programing. And I made this program complex to allow me more practice (if that makes sense). I set forth the following requirements

1. ask for the coefficients (and make sure it doesn't equal 0 for the first term)
2. display the equation in an easy to read format
3. calculate the result of the discriminant to determine the appropriate action
4. solve for the roots using the quadratic formula
5. display the results.

It works just as it should, but is there a better way to do it? Or some thing that I should have done differently?

Thanks for your help!

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

float aCoeff, bCoeff, cCoeff, discriminant, posRoot, negRoot = 0;

// This function asks for the leading coefficients of the equation,
// and stores the numbers into variables available to the entire program.
void ask()
{
	cout << "What is the leading coefficient of X^2? : ";
	cin >> aCoeff;

	// The quadratic formula doesn't work for first order equations.
	while (aCoeff == 0)
	{
		cout << "It may not equal zero, please try again. :";
		cin >> aCoeff;
	}

	cout << "What is the leading coefficient of X^1? : ";
	cin >> bCoeff;
	cout << "What is the leading coefficient of X^0? : ";
	cin >> cCoeff;
}

// This function displays the equation in an easy to read format.
void displayEquation()
{
	// For the first term, if the variable equals one do not display the value.
	// Because of the while statement in ask(), aCoeff can not equal zero.
	if (aCoeff == 1)
		cout << "\nYou entered: X^2";
	else
		cout << "\nYou entered: " << aCoeff << "X^2";

	// For the second term, if the variable equals one do not display the value.
	// If the term equals zero, the variable will not apply to any of the if statements.
	if (bCoeff == 1)
		cout << "X";
	if (bCoeff > 0)
		cout << "+" << bCoeff << "X";
	if (bCoeff < 0)
		cout << bCoeff << "X";

	// For the final term it doesn't matter if it equals one,
	// only that if it equals zero, it doesn't display the term.
	if (cCoeff > 0)
		cout << "+" << cCoeff << "=0\n";
	else if (cCoeff < 0)
		cout << cCoeff << "=0\n";
	else
		cout << "=0\n";
}

// This function calculates the roots of the equation using the quadratic formula.
void quad(float a, float b, float c)
{
	// The value of discriminant determines if there is an answer, and if there is how many.
	discriminant = (pow(b,2)-4*a*c);
	posRoot = ((-b+sqrt(discriminant))/(2*a));
	negRoot = ((-b-sqrt(discriminant))/(2*a));

	// Displaying the discrimiant for trouble shooting purposes.
	cout << "discriminant = " << discriminant << endl;

	if (discriminant < 0)
		cout << "\nThere are no real roots for that expression." << endl;

	// If the discriminant equals zero there is only one answer.
	else if (discriminant == 0)
		cout << "\nThe root is " << posRoot << endl;

	else
		cout << "\nThe roots are " << negRoot << " and " << posRoot << endl;
}

int main()
{
	cout << "***QUADRATIC ROOT FINDER VER 0.4***\n\n";
	
	ask();
	displayEquation();
	quad(aCoeff, bCoeff, cCoeff);

	// Get rid of any character in the input stream, so that it doesn't close automatically.
	cin.ignore();
	return 0;
}

[Chervil]

Generally, it is better to avoid the use of global variables. With few exceptions I'd always use double rather than float.

Also, I'd avoid calculating the square root when the value is negative. I think that is what is meant by "calculate the result of the discriminant to determine the appropriate action".

Perhaps the most tricky part of this program is the displayEquation() function. As a matter of display style, I'd prefer to put spaces between each term in the equation. Note the use of fabs().

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


// This function asks for the leading coefficients of the equation,
// and stores the numbers into variables available to the entire program.
void ask(double & aCoeff, double & bCoeff, double & cCoeff)
{
    cout << "What is the leading coefficient of X^2? : ";
    cin >> aCoeff;

    // The quadratic formula doesn't work for first order equations.
    while (aCoeff == 0)
    {
        cout << "It may not equal zero, please try again. :";
        cin >> aCoeff;
    }

    cout << "What is the leading coefficient of X^1? : ";
    cin >> bCoeff;
    cout << "What is the leading coefficient of X^0? : ";
    cin >> cCoeff;
}

// This function displays the equation in an easy to read format.
void displayEquation(double aCoeff, double bCoeff, double cCoeff)
{
    // For the first term, if the variable equals one do not display the value.
    // Because of the while statement in ask(), aCoeff can not equal zero.

    cout << "\nYou entered: ";

    if (aCoeff == 1.0)
        cout << "X^2";
    else if (aCoeff == -1.0)
        cout << "-X^2";
    else
        cout << aCoeff << "X^2";

    // For the second term, if the variable equals one do not display the value.
    // If the term equals zero, the variable will not apply to any of the if statements.
    if (bCoeff == 1.0)
        cout << " + X";
    else if (bCoeff == -1.0)
        cout << " - X";
    else if (bCoeff > 0)
        cout << " + " << bCoeff << "X";
    else if (bCoeff < 0)
        cout << " - " << fabs(bCoeff) << "X";

    // For the final term it doesn't matter if it equals one,
    // only that if it equals zero, it doesn't display the term.
    if (cCoeff > 0)
        cout << " + " << cCoeff;
    else if (cCoeff < 0)
        cout << " - " << fabs(cCoeff);

    cout << " = 0" << endl;;
}

// This function calculates the roots of the equation using the quadratic formula.
void quad(double a, double b, double c)
{
    // The value of discriminant determines if there is an answer, and if there is how many.
    double discriminant = (pow(b,2) - 4*a*c);

    // Displaying the discrimiant for trouble shooting purposes.
    cout << "discriminant = " << discriminant << endl;

    if (discriminant < 0)
    {
        cout << "\nThere are no real roots for that expression." << endl;
        return;
    }

    double posRoot = (-b + sqrt(discriminant)) / (2*a);
    double negRoot = (-b - sqrt(discriminant)) / (2*a);

    // If the discriminant equals zero there is only one answer.
    if (discriminant == 0)
        cout << "\nThe root is " << posRoot << endl;
    else
        cout << "\nThe roots are " << negRoot << " and " << posRoot << endl;
}

int main()
{
    cout << "***QUADRATIC ROOT FINDER VER 0.4***\n\n";

    double a, b, c;

    ask(a, b, c);
    displayEquation(a, b, c);
    quad(a, b, c);

    // Get rid of any character in the input stream, so that it doesn't close automatically.
    cin.ignore();
    cin.ignore();
    return 0;
}

[mLindroth]

Awesome, thank you. I really wanted to have whitespace, but I didn't know how to get rid of the negative sign. In a small program, what are the benefits of not using global variables even though they are just keeping the same name? I was trying to save it the trouble of having to move the value around.

[Chervil]

Well, three of your variables, discriminant, posRoot, negRoot are used in only one function. There is clearly no benefit in these being global. That suggests to me that by default, you are placing variables in the global namespace whether they need to be or not. I would take the opposite stance. Keep the scope of the variables limited as much as possible.

What are the issues? Well, in a very small program you can probably manage. The problem is one of starting out with a bad practice which will later cause difficulties as you progress to more complex programs.

Even here, the "trouble of having to move the value around" can be seen from the opposite perspective; by passing the variables as parameters, it is very clear what are the inputs to the function, which helps with understanding the code.

Myself, I've coded in languages where ALL the variables were always global, that was the nature of that particular language. Although it can be managed, it is a great relief to use a language where local variables are possible.

http://c2.com/cgi/wiki?GlobalVariablesAreBad