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

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.