Constructive criticism for quadratic formula calculator needed.

Hello.
Please give me constructive criticism about my quadratic formula calculator.
I would appreciate any opinions about it.

Thank you.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
#include <iostream>
#include <string>
#include <cmath>

using namespace std;

int main()

{
    unsigned int main;
    string use_again;
    long double a_var;
        a_var = 0;
    long double b_var;
        b_var = 0;
    long double c_var;
        c_var = 0;
    long double b_sqrd;
        b_sqrd = 0;
    long double root_var;
        root_var = 0;
    long double dividend;
        dividend = 0;
    long double divisor;
        divisor = 0;
    long double res;
        res = 0;
    cout << "Quadratic equation calculator." << endl;
    cout << "ax^2 + bx + c = 0." << endl;
    cout << "Uses the quadratic formula" << endl;
    cout << endl;
        do {
            cout << "Input the number left of the action." << endl;
            cout << "Input 1: -b +..." << endl;
            cout << "Input 2: _b -..." << endl;
                cin >> main;
                    if ( main == 1 )
                        {
                            cout << "Input the (a) value:" << endl;
                                cin >> a_var;
                            cout << "Input the (b) value:" << endl;
                                cin >> b_var;
                            cout << "Input the (c) value:" << endl;
                                cin >> c_var;
                                    b_sqrd = pow ( b_var, 2 );
                                    root_var = sqrt ( b_sqrd - 4 * a_var * c_var );
                                    dividend = -1 * b_var + root_var;
                                    divisor = 2 * a_var;
                                    res = dividend/divisor;
                                        cout << res << "." << endl;
                        }
                    else if ( main == 2 )
                        {
                            cout << "Input the (a) value:" << endl;
                                cin >> a_var;
                            cout << "Input the (b) value:" << endl;
                                cin >> b_var;
                            cout << "Input the (c) value:" << endl;
                                cin >> c_var;
                                    b_sqrd = pow ( b_var, 2 );
                                    root_var = sqrt ( b_sqrd - 4 * a_var * c_var );
                                    dividend = -1 * b_var - root_var;
                                    divisor = 2 * a_var;
                                    res = dividend/divisor;
                                        cout << res << "." << endl;
                        }
                    else
                            cout << "Invalid user input." << endl;

            cout << "Would you like to use this program again?" << endl;
            cout << "Input 'y' to use again." << endl;
                cin >> use_again;
        } while ( use_again == "y" );

    return 0;
}
Last edited on
a) initialize variable when you are declaring them:
1
2
3
long double a_var(0);
//or
long double a_var = 0;

b) Make program correctly guess number of roots and output them together (and do not crash when discriminant less than 0)
c) use b*b instead of pow(b, 2) — it is faster that way.
Hello MiiNiPaa,

Thank you for the quick reply.

While I do I understand and taking into account, your 1st and 3rd point, I do not understand your 2nd point; therefore: could you explain to me: what you mean?

Is the discriminant b^2 - 4ac? If it is less than 0, the quadratic equation has no real solution.

What do you mean by guess number of roots?

Thank you.
It should tell user if equation has hoots and how many:
If D < 0 — there is no roots
If D = 0 — There is one root which equals to −b / 2a (Actually 2 equal roots, but it looks ugly on the output)
If D > 0 — There is two roots.

Your program does not show both roots (if they exist) and it will crash if there is no roots at all.
Remember: used doesn't know if equation has roots, he wants your program to tell him if it has roots and what they are. If your program crashes he will think that program is unreliable and will stop using it.
I modified your program:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
#include <iostream>
#include <string>
#include <vector>
#include <limits>
#include <cmath>

std::vector<long double> solve(long double a, long double b, long double c)
{
    std::vector<long double> roots;
    long double discriminant = b*b - 4*a*c;
    if( std::fabs(discriminant) < std::numeric_limits<long double>::epsilon() ){
        roots.emplace_back( (-b)/(2*a) );
    } else if(discriminant > 0) {
        long double d_root( std::sqrt(discriminant) );
        roots.emplace_back( (-b - d_root)/(2*a) );
        roots.emplace_back( (-b + d_root)/(2*a) );
    }
    return roots;
}

template <typename T>
inline T validated_input()
{
    T value;
    while( !(std::cin >> value) ) {
        std::cout << "Wrong input, try again: ";
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    }
    return value;
}

int main()
{
    std::string use_again;
    std::cout << "Quadratic equation calculator.\n" <<
                 "ax^2 + bx + c = 0.\n" <<
                 "Uses the quadratic formula" << std::endl;
    do {
        std::cout << "Input the (a) value: ";
        long double a( validated_input<long double>() );
        std::cout << "Input the (b) value: ";
        long double b( validated_input<long double>() );
        std::cout << "Input the (c) value: ";
        long double c( validated_input<long double>() );

        std::vector<long double> roots( solve(a, b, c) );
        switch(roots.size()) {
            case 0:
                std::cout << "There is no roots";
                break;
            case 1:
                std::cout << "The root is: " << roots[0];
                break;
            case 2:
                std::cout << "The roots are: " << roots[0] <<
                             " and " << roots[1];
                break;
            default:
                std::cout << "Something gone horribly wrong";
        }
        std::cout << std::endl;

        std::cout << "Would you like to use this program again?\n" <<
                     "Input 'y' to use again: ";
        std::cin >> use_again;
    } while ( use_again == "y" || use_again == "Y" );
    return 0;
}

You still can:
* Add input validation, so program will not go haywire if user enters text as input
* Make output pretty for case where there only one root
* Make program take both 'y' and 'Y'

Disregard that, I made this already.
Last edited on
MiiNiPaa,

My program does not crash, it outputs nan. if the discriminant is less than 0.

You can also use either -b +..., or -b -... (quadratic equations).

Are you telling me, I should make my program not output nan.

So if the discriminant is less than zero, I should cout that?
Last edited on
Yes. Look at that from the user's point of view: he enters some numbers and program gave hime some nonsense. What is it: normal behavior or error? What does it mean?
Second problem arises when you want ot check if equation have some arbitrary number as one of the roots — you have to run program twice in most cases because it can show you only one root at once.
Third and main problem: mistype and enter 'e' when asked for (a).
The choice of using either of the two quadratic equations (-b+..., or -b-...) was by design, so therefore: I am going to keep it.

In any event, I am the only one using the program, and so far: the calculations that the program has done are all correct.

"Third and main problem: mistype and enter 'e' when asked for (a)."
- What do you mean by that?
If the discriminant is = 0, both options (-b+... or -b-...) will calculate the value of the variable.
Also: nan. does mean "not a number", therefore: the program is not crashing.

nan., in my opinion is acceptable.
I could also cout in the introduction to the program, that nan. means the quadratic equation has no real solutions.
When "Input the (a) value:" shows, enter letter instead of numbers: it will immideatly exits.

Good program is:
a) Error-prone: It shouldn't go mad if somebody will make a mistake (enter comma as decimal separator for example. I do that often)
b) If it designed to be interactive, it should be descriptive: it should tell what is wrong and what different answers mean.
c) Code-reusable: program should be divided into small parts which can be used multiple times. In my change I moved equation solving into function, I can call. And this validated_input() function was copied from another project. I could easily change my code to suit your needs:
he choice of using either of the two quadratic equations (-b+..., or -b-...) was by design,
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
//Your do loop. Using call to my function
        do {
            cout << "Input the number left of the action." << endl;
            cout << "Input 1: -b -..." << endl;
            cout << "Input 2: _b +..." << endl;
                cin >> main;
            if(main == 1 || main == 2) {
                cout << "Input the (a) value:" << endl;
                cin >> a_var;
                cout << "Input the (b) value:" << endl;
                cin >> b_var;
                cout << "Input the (c) value:" << endl;
                cin >> c_var;
                vector<long double> roots( solve(a,b,c) );
                switch(roots.size()) {
                    case 0:
                        std::cout << "There is no roots" << std::endl;
                        break;
                    case 1:
                        std::cout << "The root is: " << roots[0] << std::endl;
                        break;
                    case 2:
                        std::cout << "The root is: " << roots[main] << std::endl;
                        break;
                }
            }  else
                cout << "Invalid user input." << endl;

            cout << "Would you like to use this program again?" << endl;
            cout << "Input 'y' to use again." << endl;
                cin >> use_again;
        } while ( use_again == "y" );


EDIT:
If the discriminant is = 0, both options (-b+... or -b-...) will calculate the value of the variable.

But the user doesn't know if two roots are the same. He will input coefficients again, only to find that it was unnessesary and second root is the same.
Last edited on
Ok, firstly:

"But the user doesn't know if two roots are the same. He will input coefficients again, only to find that it was unnessesary and second root is the same."

- I do not see this is an issue, however I may change it.

Secondly:

"a) Error-prone: It shouldn't go mad if somebody will make a mistake (enter comma as decimal separator for example."

- How do I make my program meet this requirement?
Topic archived. No new replies allowed.