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;
}

Dani AI

Generated

Good direction from — the intent to make the code "talk for itself" is the right idea. The thread already covered portability and basic error signaling (, ) and array vs pointer clarity (). The following adds practical, durable improvements that remain useful years later: robust input parsing, correct handling of degenerate cases, support for complex roots, and a numerically stable solver.

Use line-oriented parsing for input and reject bad lines rather than relying on raw extraction. Example helper (robust, keeps responsibilities separate):

#include <string>
#include <sstream>

bool readDouble(const std::string &prompt, double &out) {
    std::string line;
    for (;;) {
        std::cout << prompt;
        if (!std::getline(std::cin, line)) return false;
        std::istringstream iss(line);
        if (iss >> out && iss.eof()) return true;
        std::cout << "Invalid number, try again.\n";
    }
}

Handle degenerate and complex cases explicitly: if a == 0 treat it as linear (bx + c = 0), and if the discriminant is negative return complex roots via std::complex<double>. For better numerical stability with real roots, compute q = -0.5*(b + sign(b)*sqrt(discriminant)) and use q/a and c/q as the two roots — this avoids catastrophic cancellation when |b| ≫ |a|.

Design the solver API for clarity: instead of printing or calling exit, return a clear result type (a small struct, std::pair<std::complex<double>, std::complex<double>>, or std::optional<std::array<double,2>> for C++17+). That lets main manage I/O, follows single-responsibility, and implements the error handling suggested while improving on it by giving richer results. Finally, compare floating-point values with a scaled epsilon (not raw == 0) and keep I/O, solving, and presentation separated for easier testing and reuse.

Recommended Answers

All 10 Replies

It's better to avoid using system("PAUSE"); (look here for more info)
You could use cin.get(); instead ...

Instead of exit(0); , consider returning a bool from your function for success or failure. Then the program can continue.

I second the cin.get() thing. It probably doesn't really matter in this case, but it's something to consider.

Ok, great... I didn't know that system("pause"); is very expensive

1.suspend your program
2.call the operating system
3. open an operating system shell (relaunches the O/S in a sub-process)
4. the O/S must now find the PAUSE command
5.allocate the memory to execute the command
6.execute the command and wait for a keystroke
7.deallocate the memory
8.exit the OS
9. resume your program

Instead of exit(0); , consider returning a bool from your function for success or failure. Then the program can continue.

Yup, I forgot to mention that, but generally it isn't recommended to use the exit(0); function ...

I second the cin.get() thing. It probably doesn't really matter in this case, but it's something to consider.

I only gave him a better alternative to system("PAUSE"); ...

Why are you passing a pointer to your array in the function void quadraticFormula(double a, double b, double c, double *totalResults) ??

You can also pass an array to a function as follows: void quadraticFormula(double a, double b, double c, double totalResults[]) as an array's name can be used as a constant pointer to that array ...

Edit:: With a constant pointer to that array I mean: a constant pointer to the first element of that array ...

>> int main(void) old syntax.
Preferred is int main() > quadraticFormula(a, b ,c, &x[0]); the last argument passed is same as passing x only. Since a name of array is pointer to first element.
Hence use: quadraticFormula(a, b ,c, x); >>void quadraticFormula(double a, double b, double c, double *totalResults)
I suggest you to use double totalResult[] instead of double *totalResults since it clarify that it is a array that your passing rather than a pointer( Actually it is the same thing. But a person, by looking the the prototype, which I mentioned is able to foretell that the function will use some array thing rather than pointer. He will be sure to pass a array rather than a simple pointer to int)

I agree, this looks much better:

void quadraticFormula(double a, double b, double c, double totalResults[])
quadraticFormula(a, b ,c, x);

Cheers...

/**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[])
{
	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;
}

Maybe you should also add an error handler ...
e.g: If this is your input:
a: 1
b: 2
c: 2

You'll get an error ...

Maybe it's useful to first check whether (b*b-4*a*c) > 0 ...

Maybe you should also add an error handler ...
e.g: If this is your input:
a: 1
b: 2
c: 2

You'll get an error ...

Maybe it's useful to first check whether (b*b-4*a*c) > 0 ...

Good one

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.