confusion on -> operator

[E093605o]

I am implementing a linked list and I get an error in line 28 "cannot convert ‘node<int>’ to ‘node<int>*’ in assignment". I am confused - next is of type node<T>* so why is it->next of type node<T>? Also, i get "‘class linkedlist<int>’ has no member named ‘print’ list.print();" and I don't know why. Can someone explain? Thanks!

  #pragma once
#include <iostream>

template <class T>
struct node
{
    T data;
    node<T> *next;

    node(T data) : data(data), next(NULL){};
};

template <class T>
class linkedlist {
    private:
    node<T> *head;
    public:
    linkedlist() : head(NULL) {}

    template <class T>
    node<T> *iterator(){
        return this->head;
    }
    template <class T>
    void append(node<T> insert_me){
        node<T> *it = iterator();
        while(it && it->next){
            it = it->next;
        }
        it->next = &insert_me;
    }
    template <class T>
    void print(){
        for (node<T>* it = iterator(); it; it=it->next){
            if (it)
                std::cout << it->data << '\n';
        }
    }
};

[rozick1]

I don't get your error when I compile but I do get an error re T shadowing a template parameter. I don't think you need the template<class T> in the method definitions.

[seeplus]

Try this:

#include <iostream>

template <class T>
struct node {
	T data {};
	node<T>* next {};

	node(T data) : data(data) {};
};

template <class T>
class linkedlist {
private:
	node<T>* head {};

public:
	linkedlist() {};

	~linkedlist() {
		while (head) {
			const auto n { head->next };

			delete head;
			head = n;
		}
	}

	linkedlist(const linkedlist&) = delete;
	linkedlist& operator=(const linkedlist&) = delete;

	node<T>* iterator() const {
		return head;
	}

	void append(node<T>* insert_me) {
		if (auto it { iterator() }; !it)
			head = insert_me;
		else {
			while (it && it->next)
				it = it->next;

			it->next = insert_me;
		}
	}

	void print() const {
		for (auto it { iterator() }; it; it = it->next)
				std::cout << it->data << '\n';
	}
};

int main() {
	linkedlist<int> ll;

	ll.append(new node<int>(1));
	ll.append(new node<int>(2));
	ll.append(new node<int>(3));

	ll.print();
}

1
2
3

[keskiverto]

void linkedlist::append( node<T> insert_me ) {
  node<T> *it = iterator();
  while(it && it->next){
    it = it->next;
  }
  it->next = &insert_me;
}

The insert_me is a by value parameter.
It is essentially a local variable with automatic lifetime.
It is created and initialized on function call and destructed at the end of the function's body.
It is a big mistake to store address of soon-to-die object into pointer that outlives the function call.

[E093605o]

thanks for your answers. Can someone explain why I get the compiler error? I use gcc 11.2

[Peter87]

dvdlly wrote:

Can someone explain why I get the compiler error?

Either you are not looking at the first error messages in the list or you have not posted the exact code that gives you those errors you mentioned (it's obviously not all the code).

[kbw]

Method append should take a value as its parameter, not a link. The links are internal details that users shouldn't see.

As for the compile error, do you still get something when you remove those extra template specs from the methods?

[E093605o]

I changed it to

template <class T>
    void append(node<T>& insert_me){
        node<T> *it = iterator();
        while(it && it->next){
            it = it->next;
        }
        it->next = &insert_me;
    }

Now it compiles and seems to work, thanks!

[seeplus]

seems to work

This would depend upon how append() is called...

[Peter87]

Imagine if the user of the class did the following:

node<int> newNode(3);
ll.append(newNode);

Then you're in trouble if newNode goes out of scope (and is destroyed) while ll is still in use.

It'll also lead to problems in the destructor because it'll try to use delete on something that was not created with new.

It's less error-prone if you do what kbw said. Let the user pass a value as argument and create the node objects internally. Don't expose them.

int newValue(3);
ll.append(newValue); // No problems here! 

[seeplus]

seems to work

C++ is one of those languages that just because some code at some point in time produces an expected output doesn't mean that the code is 'correct' or 'working' properly. This is especially true when using memory addresses...