Marcus
Marcus

Reputation: 1

Can't figure out why this isn't working

I'm a student taking c++ and I have to make a quadratic formula solver this week. I'm supposed to just use functions in the main block of code to get the output. This is what I have so far:

#include <iostream>
#include <cmath>
using namespace std;
double a, b, c, x1, x2;
char choice, response, mychar;
double disc = (b*b - 4 * a*c);

void GetCoefficients(double a, double b, double c)
{
    cout << "Enter the coefficients of your quadratic equation (a, b, c): ";
    cin >> a, b, c;
}

bool ComputeRoots(double a, double b, double c, double x1, double x2)
{
    
    if (disc > 0)
    {
        x1 = (-b + sqrt((b*b) - 4 * a*c)) / 2 * a;
        x2 = (-b - sqrt((b*b) - 4 * a*c)) / 2 * a;
        return true;
    }
    if(disc == 0)
    {
        x1 = x2 = (-1 * b) / (2 * a);
        return true;
    }
    else 
    {
        cout << "'a' cannot be zero. That is not a quadratic equation.";
        return false;
    }
}

char PromptToContinue()
{
    char mychar;
    cout << "Would you like to solve another quadratic equation (Y, N): ";
    cin >> mychar;
    return mychar;
}

void PrintRoots(double x1, double x2)
{
    if (disc > 0)
    {
        cout << "The roots are: " << x1 << ", " << x2;
    }
    
    if (disc == 0)
    {
        cout << "The single root is: " << x1;
    }
}

void main()
{

    do
    {
        GetCoefficients(a, b, c);
        if (ComputeRoots(a, b, c, x1, x2))
        {
            PrintRoots(x1, x2);
        }
        choice = PromptToContinue();
    } while (choice != 'n' || 'N');
    system("pause");
}

I'm sure I have multiple problems with my code, but I've been staring at this for hours and have no idea why its not working. Any insight would be awesome.

The example output I'm supposed to get is this (with the values after the colon being the user input):

Enter the coefficients of your quadratic eqn (a, b, c): 2 3 4
The roots are complex.

Would you like to solve another quadratic equation (Y, N): y
Enter the coefficients of your quadratic eqn (a, b, c): 1 2 1

The single root is: -1

Would you like to solve another quadratic equation (Y, N): y
Enter the coefficients of your quadratic eqn (a, b, c): 0 5 6

'a' cannot be zero. That is not a quadratic equation.

Would you like to solve another quadratic equation (Y, N): y
Enter the coefficients of your quadratic eqn (a, b, c): 5 25 5

The roots are: -0.208712, -4.79129

Would you like to solve another quadratic equation (Y, N): n
Press any key to continue . . .

The output I'm getting is this:

Enter the coefficients of your quadratic equation (a, b, c): 2 3 4
The single root is: 0Would you like to solve another quadratic equation (Y, N):
Enter the coefficients of your quadratic equation (a, b, c): The single root is:
 0Would you like to solve another quadratic equation (Y, N):

Upvotes: 0

Views: 229

Answers (4)

Deepankar Singh
Deepankar Singh

Reputation: 674

Probably you're confused with using local and global variables.Here you've specified all your variable as global so you don't need to pass variables in function parameter(keep in mind, using many global variables is a bad programming practice).Your code has lot of semantic problems

#include <iostream>
#include <cmath>
using namespace std;
double a, b, c, x1, x2;
char choice, response, mychar;
double disc = (b*b - 4 * a*c); // here only declare disc like `double disc;`  create a function for calculating disc like
/* void caldisc()
{
disc = (b*b - 4 * a*c);
} call this caldisc() in main after GetCoefficients()*/

    void GetCoefficients(double a, double b, double c) // passing parameters double a, double b, double c means your creating local copy of a b and c and getting input in it not in global a b c
    {
        cout << "Enter the coefficients of your quadratic equation (a, b, c): ";
        cin >> a, b, c;   // use cin>>a>>b>>c;
    }

    bool ComputeRoots(double a, double b, double c, double x1, double x2) // dont pass parameters here and use directly it in main() like ComputeRoots();
    {

        if (disc > 0)
        {
            x1 = (-b + sqrt((b*b) - 4 * a*c)) / 2 * a;
            x2 = (-b - sqrt((b*b) - 4 * a*c)) / 2 * a;
            return true;
        }
        if(disc == 0)
        {
            x1 = x2 = (-1 * b) / (2 * a);
            return true;
        }
        else 
        {
            cout << "'a' cannot be zero. That is not a quadratic equation.";  // this block will get executed when disc<0 regardless of a. ZeroCheck of a should be done in upper blocks.
            return false;
        }
    }

    char PromptToContinue()
    {
        char mychar;       // here you're creating local copy of mychar and returning it in last line.Thus local copy of mychar will return garbage. remove this line
        cout << "Would you like to solve another quadratic equation (Y, N): ";
        cin >> mychar;
        return mychar; // remove this change function return type as void
    }

    void PrintRoots(double x1, double x2) // no need to pass x1 and x2 change function call in main too.
    {
        if (disc > 0)
        {
            cout << "The roots are: " << x1 << ", " << x2;
        }

        if (disc == 0)
        {
            cout << "The single root is: " << x1;
        }
    }

now in main dont use choice. check do-while condition using mychar !='N' // means loop will be executed if you pass anything other than N

Upvotes: 0

Quentin
Quentin

Reputation: 63154

One obvious problem is that void GetCoefficients(double, double, double)'s parameters are passed by value, when you obviously want them passed by reference. As-is, you're copying them inside your function, reading user input into these copies, then dropping them all when returning.

Edit: void main() is incorrect, main should have type int main(int, char**) or int main().

while (choice != 'n' || 'N'); doesn't do what you expect : it checks whether choice is different from 'n', then does a boolean or with the literal 'N'. Since 'N' is non-zero, the condition is always true. The correct syntax is (choice != 'n' && choice != 'N').

Your program is also quite weirdly shaped overall, but I guess it will get better. For example you use only global variables, which should be avoided when possible. void PrintRoots() is funny in that it takes half of its parameters by its parameters list, and grabs the other half from global variables.

Upvotes: 4

M.M
M.M

Reputation: 141658

This code:

double a, b, c, x1, x2;
double disc = (b*b - 4 * a*c);

sets disc to zero. In C++, expressions are computed using the values of the variables at the time the expression is encountered. This line does not set up a formula that will be used to compute disc any time you use disc later in the program.

(Note: since double a, b, c, x1, x2; is global, those values are initialized to 0.0, and so the calculation for disc ends up with 0.0 too).

For example, you go on to do:

bool ComputeRoots(double a, double b, double c, double x1, double x2)
{
    if (disc > 0)

however disc is still zero, since you set it to zero at the start of the program as mentioned earlier, and you haven't changed it.

To set up a situation where a value is computed as a result of other input values you need to write a function, for example:

double disc(double a, double b, double c) { return b*b - 4*a*c; }

Upvotes: 1

cin >> a, b, c;

is wrong and does not do what you believe (read about the comma operator).

You might want

cin >> a >> b >> c;

at least

Compile with all warnings & debug info (g++ -Wall -Wextra -g). Then use the debugger (gdb) e.g. to run your program step by step.

More generally, read a lot more documentation (on C++) about the functions and operators you are using (e.g. this)

Upvotes: 2

Related Questions