Polynomial program

Question

The program basically takes a polynomial and does some simple calculations with it.

With a couple of hours of work I managed to get my first console application to work (aside from the obligatory "Hello, World!", of course). However, I'm in total doubt that the style and habits I've used are even close to being correct.

#include <iostream>
#include <fstream>
#include <string>
#include <algorithm>
#include <math.h>
#include <cmath>

using namespace std;

class Polynomial {
    float a, b, c, value;

public:
    Polynomial ();
    void functionDefine(Polynomial poly);
    void functionValue(Polynomial poly);
    void functionZero(Polynomial poly);
};

namespace Utils {
    void mainMenu(Polynomial poly);
    void skipToMenu();
    string save(string str);
    int exitProgram();

    void mainMenu(Polynomial poly) {
        int choice = 0;

        cout << "   ---------- Main Menu ----------" << endl;

        cout << "1: ---- Define New Polynomial ----" << endl;
        cout << "2: ------ Solve for x-value ------" << endl;
        cout << "3: --------- Find roots ----------" << endl;
        cout << "4: ------------ Exit -------------" << endl << endl;

        cout << "Enter the value of the submenu you want to enter: ";

        cin >> choice;
        cout << endl;

        switch(choice) {

            case 1:
                poly.functionDefine(poly);
                break;

            case 2:
                poly.functionValue(poly);
                break;

            case 3:
                poly.functionZero(poly);
                break;

            case 4:
                exitProgram();
                break;
        }
    }

    void skipToMenu() {
        cout << "Press enter to return to the main menu."<< endl << endl;
        system("pause > nul");

    }

    string save(string str)  {
        string saved = "Yes or no"; // To do.
        return saved;
    }

    int exitProgram() {
        return 0;
    }
}

Polynomial::Polynomial() {
    a = 0;
    b = 0;
    c = 0;

}

void Polynomial::functionDefine (Polynomial poly) {
    char currentChar = 'a';

    cout << "---- Define New Polynomial ----" << endl;

    for (int i = 0; i < 3; i++) {
        cout << "Enter a value for " << currentChar << ": ";

        switch(i)
        {
            case 0:
                cin >> poly.a; break;
            case 1:
                cin >> poly.b; break;
            case 2:
                cin >> poly.c; break;
        }

        if (!cin) {
            cout << "Please enter a number instead." << endl;
            system("pause");
        }

        currentChar = static_cast<char>(currentChar + 1);

    }

    cout << endl;
    cout << "The function is: f(x) = " << poly.a << "x^2 + " << poly.b << "x + " << poly.c << "." << endl << endl;

    Utils::skipToMenu();
    Utils::mainMenu(poly);

}

void Polynomial::functionValue(Polynomial poly) {
    int input;

    cout <<"------ Solve for x-value ------" << endl << endl;

    cout << "Enter the x-value that you would like to evaluate the equation for: ";
    cin >> input;

    value = poly.a * pow(input, 2) + poly.b * input + poly.c;
    cout << "The value of the function for x = " << input << " is " << value << endl << endl;

    Utils::mainMenu(poly);
}

void Polynomial::functionZero(Polynomial poly) {
    float d = pow(poly.b, 2) - 4 * poly.a * poly.c;

    if (d > 0) {

        float result = ((-poly.b) + sqrt(abs(d))) / (2 * poly.a);
        float secondResult = (-poly.b - sqrt(abs(d))) / (2 * poly.a);
        cout << "The polynomials roots are at x = " << result << " and x = " << secondResult << "." << endl << endl;
    }

    else if (d == 0) {
        float result = ((-b) + sqrt(abs(d))) / (2 * a);
        cout << "The polynomials root is at x = " << result << "." << endl << endl;
    }

    else {
        cout << "The polynomial has no roots." << endl << endl;
    }

    Utils::mainMenu(poly);
}

int main () {
    Polynomial poly;
    Utils::mainMenu(poly);

    return 0;
}

I was told it would be a good idea to create different namespaces for my "calculations" and my "utilities". Creating a class with the utility functions wouldn't make sense as there would never be created an instance of this...

I'm open to all the critique you have. If anybody wants the Pastebin link it's here.

Yuushi · Accepted Answer · 2013-06-06 07:15:19Z

As others have mentioned, you should get into the habit of breaking up function definitions and declarations (effectively, function signatures and implementations) into separate files. In this case, the header file should look something like:

class Polynomial 
{
    float a, b, c, value;

public:
    Polynomial ();
    void functionDefine(Polynomial poly);
    void functionValue(Polynomial poly);
    void functionZero(Polynomial poly);
};

namespace Utils
{
    void mainMenu(Polynomial poly);
    void skipToMenu();
    string save(string str);
    int exitProgram();
}

This is then included in the file with the class/function implementations - so if we called it something like Poly.h, in the implementation file we'd have:

#include "Poly.h"

Let's go through the Polynomial class definition, because there's some room for improvement there. Firstly, the way it currently stands, they're effectively all just static functions that take a Polynomial as a parameter. This goes against the idea of what classes are meant to do. Classes are supposed to be a grouping of functions and data together, where the functions act (ideally) only on the data contained within an instance of the class. This means that you construct a given Polynomial, and then call methods on that particular instance. I'd rewrite this as follows:

class Polynomial 
{
    float a, b, c;

public:
    Polynomial();
    Polynomial(float a_, float b_, float c_);
    void functionValue() const;
    void functionZero() const;
};

There's a few new things here. Firstly, I've removed value because it doesn't really get used anywhere, and it isn't really an attribute itself of the given Polynomial. Secondly, I've replaced the functionDefine with a second constructor, which takes 3 float parameters. Note also the const on the end of functionValue() and functionZero(). This says that calling these functions should not change the values in the current polynomial instance, which they shouldn't (finding the roots of a polynomial shouldn't modify any of the a, b, or c values). The constructors are then defined as:

Polynomial::Polynomial()
  : a(0), b(0), c(0) 
{ }

Polynomial::Polynomial(float a_, float b_, float c_)
  : a(a_), b(b_), c(c_)
{ }

The current program structure passes a given Polynomial around everywhere, it's threaded through the entire program. We're going to remove a lot of that because it's mostly unneeded. Firstly, we're going to change the mainMenu function a bit:

void mainMenu() 
{
    int choice = 0;
    Polynomial poly;
    bool finished = false;

    while(!finished) {
        cout << "   ---------- Main Menu ----------" << endl;
        cout << "1: ---- Define New Polynomial ----" << endl;
        cout << "2: ------ Solve for x-value ------" << endl;
        cout << "3: --------- Find roots ----------" << endl;
        cout << "4: ------------ Exit -------------" << endl << endl;

        cout << "Enter the value of the submenu you want to enter: ";

        cin >> choice;
        cout << endl;

        switch(choice) {
            case 1:
                poly = functionDefine();
                break;
            case 2:
                poly.functionValue();
                break;
            case 3:
                poly.functionZero();
                break;
            case 4:
                finished = true;
                break;
            default:
                std::cout << "That is not a valid selection\n\n";
                break;
        }
    }
}

There are a few changes here. Firstly, we create a Polynomial instance up the top of this function. Since we always return to the main menu after a selection (except on exit), this will never falls out of scope and can be utilized for the entire program. When we want to assign a new value to it, we use poly = functionDefine(). When we want to calculate some value at a given point or find zeros, we call the functions poly.functionValue() and poly.functionZero() on it. This is all put into a while loop so that we never exit the mainMenu function until we are done. Finally, we've added a default value to the switch statement which will simply print an error and return us to the mainMenu if the user selects some value outside of 1 - 4.

The functions themselves are pretty similar, except functionDefine is now a free-standing function, while functionZero and functionValue operate on a polynomial instance:

Polynomial functionDefine() 
{
    static const int maxDegree = 2;
    float a, b, c;
    char currentChar = 'a';
    int filled = 0;

    cout << "---- Define New Polynomial ----" << endl;

    while(filled < maxDegree + 1) {
        cout << "Enter a value for " << static_cast<char>(currentChar + filled) 
             << ": ";

        switch(filled)
        {
            case 0:
                cin >> a; 
                break;
            case 1:
                cin >> b; 
                break;
            case 2:
                cin >> c; 
                break;
        }

        if(!cin) {
            cout << "Please enter a number instead." << endl;
            cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(),'\n');
        } else {
            ++filled;
        }
    }

    cout << endl;
    cout << "The function is: f(x) = " << a << "x^2 + " << b << "x + " << c << "." << endl << endl;

    return Polynomial(a, b, c);
}

functionDefine now returns an instance of a Polynomial, and also makes use of the suggested fix from @h4ck.b0x7.

void Polynomial::functionValue() const
{
    int input;
    double value;

    cout <<"------ Solve for x-value ------" << endl << endl;

    cout << "Enter the x-value that you would like to evaluate the equation for: ";
    cin >> input;

    value = a * pow(input, 2) + b * input + c;
    cout << "The value of the function for x = " << input << " is " << value << endl << endl;
}

Note that this should also do some error checking on cin >> input. It doesn't crash currently (which is good), but if the user enters a non-number, you should probably tell them and then try again.

void Polynomial::functionZero() const
{
    float d = pow(b, 2) - 4 * a * c;

    if (d > 0) {

        float result = ((-b) + sqrt(abs(d))) / (2 * a);
        float secondResult = (-b - sqrt(abs(d))) / (2 * a);
        cout << "The polynomials roots are at x = " << result << " and x = " << secondResult << "." << endl << endl;
    }

    else if (d == 0) {
        float result = ((-b) + sqrt(abs(d))) / (2 * a);
        cout << "The polynomials root is at x = " << result << "." << endl << endl;
    }

    else {
        cout << "The polynomial has no roots." << endl << endl;
    }
}

Finally, exitMenu() and skipToMenu() are gone, as they didn't really need to be there. Our main function is then:

int main () 
{
    Utils::mainMenu();
    return 0;
}

As a final note, I think the names could do with a bit of work - they're not particularly descriptive. Perhaps solveForX() instead of functionValue(), and findZeros() instead of functionZero().

@wizH, this is what Code Review is all about. :-) Feel free to come back whenever you have more programs needing a review. Not a bad start for a beginner (I'd barely even consider myself intermediate). — Jamal♦, Jun 3 '13 at 17:02

Community · Accepted Answer · 2017-05-23 12:40:59Z

First, your program has no need for these Libraries:

#include <fstream>
#include <algorithm>
#include <math.h>

The <math.h> is redundant due to the use of <cmath>. <cmath> is the C++ way. Another example is <cstdlib> instead of <stdlib.h>. The C libraries are deprecated in C++, mainly they're just for compatibility. <fstream> is for file streams, your program doesn't use any.

Secondly, I would avoid using system("pause"), it is EXTREMELY resource heavy compared to what it does. There are many ways to implement a pause into your program, the easiest however do not allow you to press any key to continue, you have to generate a new line to continue (Press Enter). If you include the <limits> library you can do something like the following:

#include <iostream>
#include <limits>

int main()
{
    int test;

    std::cout << "Var: ";
    std::cin >> test;

    std::cin.clear();
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(),'\n');
    std::cin.get();
    return 0;
}

Make sure you pay attention to these lines:

std::cin.clear();
std::cin.ignore(std::numeric_limits<std::streamsize>::max(),'\n');
std::cin.get();

The first 2 lines clear the buffer, which is important. Without those lines there is still a '\n' character in the buffer from the previous input. When your program reaches:

std::cin.get();

It will read that '\n' character and then continue. By clearing the buffer first, the program will get input until it receives a '\n' from the user by pressing enter.

I usually create a void function out of the first 2 lines.

Lastly, you should read this: Why is “using namespace std;” considered bad practice?

Hope that helps a bit.

Thanks for the input. I included fstream because I was saving all the calculations to a textfile, just removed that functionality again :) Well, I guess I won't use system("pause") in the future again then ^^ Still interested in having the style/habits reviewed by someone if possible! — wizH, Jun 1 '13 at 10:59

h4ck.b0x7 · Accepted Answer · 2013-06-03 00:08:05Z

I don't think your style here is bad, although, if I were you I would move your class into a separate header file. Use function prototypes at the top of this file and move the Utils namespace below main.

One thing I did notice though is you should use initialization lists for your constructors.

You can remove:

Polynomial::Polynomial() {
    a = 0;
    b = 0;
    c = 0;

}

And in your class declaration change the constructor line to:

Polynomial () : a(0), b(0), c(0) {}

Just my 2 cents.

Linuxios · Accepted Answer · 2013-06-03 02:38:35Z

1

Don't overrefactor. Do you really need exitProgram? That already exists in the standard library as exit. Why can't you just use exit(0)?

I think a lot of C++ books teach you to make everything and anything it's own everything, but that isn't really practical in large scale applications or codebases.

answered Jun 3 '13 at 2:38

Linuxios

1414 bronze badges

\$\begingroup\$ Plus, it is useless since all it does is terminate itself, not the program. I've been told that exit() isn't preferred either, unless there's no other way of falling back to main(). \$\endgroup\$ – Jamal♦ Jun 3 '13 at 2:57
\$\begingroup\$ @Jamal: all good points, I might add them later. \$\endgroup\$ – Linuxios Jun 3 '13 at 13:31
\$\begingroup\$ @wizH: sure. Glad to help. \$\endgroup\$ – Linuxios Jun 3 '13 at 13:31

add a comment |

Alnitak · Accepted Answer · 2016-01-06 09:22:33Z

I would very strongly urge you to separate all of the input/output functionality from the Polynomial class itself.

That class should only contain the basics relating to the polynomial calculations, allowing other code to capture the input and display the output, e.g:

#include <cmath>
#include <vector>

class Polynomial {
    float a, b, c;

public:
    Polynomial();
    Polynomial(float a, float b, float c);
    void setCoefficients(float a, float b, float c);
    float evaluate(float x) const;
    std::vector<float> roots() const;
};

Polynomial::Polynomial()
    : a(0), b(0), c(0)
{}

Polynomial::Polynomial(float _a, float _b, float _c)
    : a(_a), b(_b), c(_c)
{}

void Polynomial::setCoefficients(float _a, float _b, float _c)
{
    a = _a;
    b = _b;
    c = _c;
}

float Polynomial::evaluate(float x) const
{
    return a * (x * x) + b * x + c;
}

std::vector<float> Polynomial::roots() const
{
    std::vector<float> result;
    float det = b * b - 4 * a * c;
    if (det > 0) {
         float sd = sqrt(det);
         result.push_back((-b + sd) / (2 * a));
         result.push_back((-b - sd) / (2 * a));
    } else if (det == 0) {
         result.push_back(-b / (2 * a));
    }
    return result;
}

The use of the std::vector<float> as the return type from the roots() function allows the return of 0, 1 or 2 results, depending on the coefficients.

This class can now be used for non-interactive calculations - in fact I might even build something like this into my ray tracer since I've currently got a sphere intersection test that requires a quadratic solver but I'm expecting to need that solver in some more places too.

Also note that separating out this functionality and removing the interactivity allows the class to be trivially "unit tested".

Stack Exchange Network

current community

your communities

more stack exchange communities

Polynomial program

5 Answers 5

Your Answer

Not the answer you're looking for? Browse other questions tagged c++ performance beginner mathematics or ask your own question.

Hot Network Questions

Polynomial program

5 Answers 5

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c++ performance beginner mathematics or ask your own question.

Related

Hot Network Questions