0

I am trying to create a Polynomial class .The terms of each object should be sorted for an ease of use( from higher exponent to smaller one). So i do the sorting in the addTerm method but i am having some trouble doing it right. I either create an infinite linked list or the things are not sorted properly

#include <iostream>

using namespace std;

class Polynomial {
protected:
    class Term {
    public:
        int exponent;
        int coefficient;
        Term *next;

        Term(int exp, int coeff, Term *n) {
            exponent = exp;
            coefficient = coeff;
            next = n;
        }
        friend class Polynomial;
    };

public:
    Polynomial() {
        root = NULL;
    }
    Polynomial(const Polynomial &p) {
        root = p.root;
    }
    ~Polynomial() {
        root = NULL;
    }
    void addTerm(int expon, int coeff) {

        Term *prev = root;
        Term *target = root;

        if (root == NULL) {
            Term tmp = Term(expon, coeff, NULL);
            Term *p = &tmp;
            root = p;

            return;
        }

        while (target!= NULL && target->exponent < expon) {
            prev = target;
            target = target->next;
        }

        if (prev == target){
            Term tmp = Term(expon, coeff, target;
            Term *p = &tmp;
            prev->next = p;
        }else{


        Term tmp = Term(expon, coeff, target);
        Term *p = &tmp;
        prev->next = p;

        }




    }



private:
    Term *root;
};

int main() {
    Polynomial q;
    q.addTerm(1, 4);
    q.addTerm(2, 3);
    q.addTerm(3, 4);
    //q.addTerm(1, 4);
    //q.addTerm(2, 4);
    // q.addTerm(2,4);
    //q.print();
    //cout << q;
}
Stavros Avramidis
  • 69
  • 1
  • 6
  • Separate the concerns. The polynomial class is all tangled up with the implementation of a list. Use a list type to build the polynomial type. Best would be to use a standard STL container like `std::list`. If a benighted instructor will not allow it, test your polynomial code using a standard list container to implement it, then try to roll your own list replacement. – Jive Dadson Mar 29 '18 at 19:39
  • both the main class and sub class methods should be as is .. – Stavros Avramidis Mar 29 '18 at 19:47
  • If you mean that your instructor said you must do it that way, well that's a shame. Just bear in mind that he is telling you to do it badly. Get a good book. – Jive Dadson Mar 29 '18 at 19:51
  • I meant that the we only write the inside of each method not the name and their vars...cause they pass form a grader ...Also my instuctor worked at google for V8 js engine so i must not explaining this correctly – Stavros Avramidis Mar 29 '18 at 20:02
  • I don't know what you mean, but no matter. Ask your instructor if you can use a standard list, and if not (unlikely given his credentials), if you can write a separate list class and incorporate it into polynomial. – Jive Dadson Mar 29 '18 at 20:05
  • I could do that but it works fine just addTerm has a problem.. – Stavros Avramidis Mar 29 '18 at 20:08
  • Whatever. Good luck. – Jive Dadson Mar 29 '18 at 20:15
  • @JiveDadson "*If a benighted instructor will not allow it...*" OP shouldn't be allowed anywhere near the STL list given how they're struggling with a simple singly linked list implementation. They would have no idea what they were using or what it was doing! If this situation was in the workplace and you replaced "instructor" with "boss" I would agree with dubbing them "benighted," but academia is where you need to learn from the ground up. – scohe001 Mar 29 '18 at 20:32
  • @scohe001 I am aware that many instructors teach that way. I however agree whole-heartedly with Kate Gregory. Learn from the sky down. I recommend this video very highly. https://youtu.be/YnWhqhNdYyk – Jive Dadson Mar 29 '18 at 20:38
  • @JiveDadson Touché. To each his own. – scohe001 Mar 29 '18 at 20:45

1 Answers1

1

There is a lot wrong here. I'm going to enumerate through some of what I see, but I don't think I'll be catching everything...

1. Don't use NULL

In your default constructor you set root == NULL;. Don't. See here for more.

2. Your copy constructor makes a shallow copy.

root = p.root; means that both root's are pointing to the same list! This means modifying one will modify the other which will result in some nasty surprises. You need to create a deep copy.

3. Your destructor doesn't deallocate anything!

You set root back to the null pointer again and end the function. This is the equivalent of taking the trashbag out and setting it beside the can and considering the trash cleaned. You have memory leaks!

4. Your pointers all point to bad objects!

Term tmp = Term(expon, coeff, NULL);
Term *p = &tmp;
root = p;

Yipes! You allocate a Term locally and give root its address. But after that function ends, tmp goes out of scope and gets destroyed, leaving root pointing to an invalid memory location! You need to dynamically allocate memory that will persist for the life of the object.

Community
  • 1
  • 1
scohe001
  • 15,110
  • 2
  • 31
  • 51
  • 1. So you say to use `0`? 2.I know for the time being i dont' care....:P 3.How should i do it?? 4.i tried `Term tmp = new(expon, coeff, target);` but i got an error – Stavros Avramidis Mar 29 '18 at 18:59
  • @StavrosAvramidis **1.** use `0` or `nullptr` if you can. `NULL` is deprecated, so you have no guarantee it will be valid. **3.** Every call to `new` requires a matching call to `delete` (otherwise you have leaks). You'll need to loop through and call delete on each Term you've allocated. **4.** try `Term *tmp = new Term(expon, coeff, target);` – scohe001 Mar 29 '18 at 20:33