Output operator is not producing expected outcome

Question

I am implementing a class Polynomial with private members consisting of a singly linked list for its coefficient and an int representing the highest degree. The constructor takes in a vector which represents the coefficients in the polynomial. This is my implementation of the constructor and the output operator.

#include <iostream>
#include <vector>

using namespace std;

struct Node{
    Node(int data = 0, Node* next = nullptr) : data(data), next(next) {}
    int data;
    Node* next;
};
class Polynomial{
    friend ostream& operator<<(ostream& os, const Polynomial& p);

    public:
    Polynomial(vector<int> poly){
        Node* temp = co;
        for(int i : poly){
            temp = new Node(i);
            temp = temp->next;
        }
        degree = poly.size() - 1;
    }
    private:
    Node* co;
    int degree;
};
ostream& operator<<(ostream& os, const Polynomial& p){
    Node* temp = p.co;
    int degree = p.degree;
    while(temp != nullptr){
        if(degree == 1){
            os << temp->data << "x" << " ";
        }else if(degree == 0){
            os << temp->data;
        }else{
            os << temp->data << "x^" << degree << " ";
        }
        degree--;
        temp = temp->next;
    }
    return os;
}

When I try to test my code the output was 686588744, which I assume refers to a place in memory, rather than the expected outcome of 17.

int main(){
     Polynomial p1({17});
     cout << p1;
}

Could anyone point out where I made a mistake in my code?

The first line in your streaming operator assigns the value of `p.co` to `temp`. Could you please point to the line where the `co` field is assigned a value so that I can follow what `temp` equals at that point? — JaMiT, Apr 26 '22 at 22:30
The `Polynomial` constructor is broken, leaves `co` uninitialized, and leaks all of the nodes created in it. https://godbolt.org/z/Yse1c8bsE — Retired Ninja, Apr 26 '22 at 22:31
Recommendation: Try to keep the number of concerns you have to handle at a time to the bare minimum. If you split the logic up and separate the application logic from the linked list logic, you can write and test them separately. This generally makes for quicker debugging. — user4581301, Apr 26 '22 at 22:31
@RetiredNinja I initialized `co` to a Node* but the problem still persists. Is there an issue with the for loop? — chickennuggies, Apr 26 '22 at 22:38
You current code does not initialize `co`. It initializes `temp` with `co`, but follows up by writing over `temp` with `temp = new Node(i);`. At no point is `co` ever pointed at a valid `Node`. — user4581301, Apr 26 '22 at 22:43
This question is a demonstration of [Why should I always enable compiler warnings?](https://stackoverflow.com/questions/57842756/why-should-i-always-enable-compiler-warnings) (For this code: `warning: '*this.Polynomial::co' is used uninitialized [-Wuninitialized]` on line `Node* temp = co;`) — JaMiT, Apr 28 '22 at 05:43

Vlad from Moscow · Accepted Answer · 2022-04-26T22:49:16.437

This constructor

Polynomial(vector<int> poly){
    Node* temp = co;
    for(int i : poly){
        temp = new Node(i);
        temp = temp->next;
    }
    degree = poly.size() - 1;
}

is invalid. Actually it does not build a list. Instead it produces numerous memory leaks.

For example at first a node was allocated and its address was assigned to the pointer temp and then the pointer was reassigned with the value stored in the data member temp->next that is a null pointer. So the address of the allocated node was lost.

And moreover the pointer co is leaved uninitialized.

You can rewrite it for example the following way

Polynomial( const vector<int> &poly) : co( nullptr ), degree( 0 )
{
    Node **current = &co;

    for( int i : poly )
    {
        *current = new Node(i);
        current = &( *current )->next;
    }
     
    if ( not poly.empty() ) degree = poly.size() - 1;
}

Output operator is not producing expected outcome

1 Answers1