0

I'm really new to c++ and trying construct a basic LinkedList Node just to basically understand the whole pointer/reference thing.

I'm trying to store a reference to an object being passed to the constructor inside my node Class.

I've also created a copy constructor in order to see when Nodes are being copied (for testing purposes)

Node code bellow:

#include <iostream>
using namespace std;


template<typename T>
struct Node {

    Node(const T &d): data(d), next(nullptr), prev(nullptr){
        ;
        cout << "created Node obj" << endl;
    };

    Node (const Node &node) {
        cout << "Copied Node obj" << endl;
    }

    void setNext(Node *next){ this->next = next; };
    void setPrev(Node *prev){ this->prev = prev; };
    Node* getNext(){ return next ;};
    Node* getPrev(){ return prev ;};
    T& getData(){ return  data ;};

private:
    Node *next;
    Node *prev;
    T data;

};

and also I have this basic code for testing:

#include <iostream>
#include <vector>
#include "Node.h"

using namespace std;
    int main() {
        vector <int> v1 {1};

        Node< vector<int> > n1{v1};  // (1)

        Node < Node< vector<int> > > nn1{n1};   // (2)
        Node< vector<int> > & n1ref = nn1.getData();
        return 0;
    }

so first of all using the debugger after line (1) I've seen the copy constructor of vector class is being used for no apparent reason to me.

so I've also tested with line (2) - and indeed the node inside the node is being copied - can someone please clarify why does the node & vector are being copied although I've passed them as a reference?

btw the output of running the main is:

created Node obj
Copied Node obj
created Node obj

UPDATE first of all guys thanks for the commitment, I'm taking in to consideration the notes about storing a pointer, or even a copy (which could make more sense sometimes as i see now)

but the code above is mostly for the practice of pointers/references, so at least for now I would like to stick to the reference member.

after trying @underscore_d 's suggestion to update the member type to T& I've come with the following results:

firstly the IDEA (CLION) made me update the copy constructor - now it has to update the data member in the initialization list. I'm guessing that its because a reference should be assigned upon creation - can someone approve/clarify this?

so the code is as follows:

template<typename T>
struct Node {

    Node(const T & d): data(d), next(nullptr), prev(nullptr){

        cout << "created Node obj" << endl;
    };

    Node (const Node &node): data(node.getData()) {
        cout << "Copied Node obj" << endl;
    }

    void setNext(Node *next){ this->next = next; };
    void setPrev(Node *prev){ this->prev = prev; };
    Node* getNext() const { return next ;};
    Node* getPrev() const { return prev ;};
    T& getData() const { return  data ;};

private:
    Node *next;
    Node *prev;
    T& data;

}; 

problem is now compilation fails (with the same main class) with the following error:

error: binding 'const std::vector<int>' to reference of type 'std::vector<int>&' discards qualifiers

which I don' fully understand, any help would be appreciated. and again thanks for the efforts to help - very appreciated.

Lev
  • 105
  • 2
  • 8
  • 5
    Please [abandon the use of `using namespace std;`](https://stackoverflow.com/q/1452721/10077). There is potential for name collisions with `next`, `prev`, and `vector`. I can't tell for sure if you're defining your own `vector` or if you're using `std::vector`. – Fred Larson Jul 31 '17 at 13:54
  • `why does the node & vector are being copied although I've passed them as a reference?` because you assign that reference to a non-reference member, therefore the passed object is copied into the member. If you want a reference as a member, then declare the member as a reference... although chances are for this structure it should be a pointer. See my comment to the answer for more info on how this is typically handled. – underscore_d Jul 31 '17 at 14:11
  • 3
    See [How to initialize the reference member variable of a class?](https://stackoverflow.com/questions/15403815/how-to-initialize-the-reference-member-variable-of-a-class) but then [Should I prefer pointers or references in member data?](https://stackoverflow.com/questions/892133/should-i-prefer-pointers-or-references-in-member-data). As this is basically caused by a simple typographic error - the lack of an ampersand in the member declaration - perhaps it is effectively a duplicate of one of those. – underscore_d Jul 31 '17 at 14:22
  • Having said that, do you really want to store a reference/pointer at all? Do you guarantee that the original object's lifetime exceeds that of the list element? Does it need to? Storing a value might be exactly the right thing to do. C++ prioritises value semantics. Storing by value means you know that the element owns its value and you don't have to worry about lifetime of the thing you originally passed in. If your elements are expensive to copy, then use move semantics to save cycles, though copy-elision might take care of that already. – underscore_d Jul 31 '17 at 14:33
  • 1
    Regarding your edit, [the "Should I..." post](https://stackoverflow.com/questions/892133/should-i-prefer-pointers-or-references-in-member-data) I linked above explains why reference members can be problematic if reassignment might be required. As for the error, tell us the whole thing, including line numbers, but it's presumably because your constructor tries to store its `const` argument in a non-`const` member reference, which obviously isn't allowed. I think this is fast becoming too broad to answer. – underscore_d Jul 31 '17 at 14:50
  • 1
    You do not want to store references in your list. (You can try, but that's the lesson you're going to learn from trying.) A "reference" in C++ is not like a "reference" in other languages you may be familiar with. You could say that a C++ reference is not "a thing that refers to something", it *is* the thing it refers to. – molbdnilo Jul 31 '17 at 15:10

2 Answers2

1

I have my C++ gotten rusty, but the fact that the value inside the nodes are being copied, I believe, is because you define the attribute:

T data;

That is, an object of class T. So in (1) when the node gets created, it copies the reference parameter (I mean the const T &d) into the attribute data as you declare in the constructor with data(d).

Maybe a T* data as attribute would help. In this case, you have options:

a. Change the constructor to receive a T* as parameter instead of a reference T& (and the related code properly)

b. Receive the T& in the constructor, but conver the reference to pointer internally. For this, write data(&d) as initialization in the constructor. And remember to change to T& getData(){ return *data ; } (and anything related)

About why it shows:

created Node obj
Copied Node obj
created Node obj

it is because:

  • In (1) shows the first "create Node obj", since Node< vector<int> > n1{v1}; creates the Node n1.
  • In (2), it shows the other two messages because:
    • Node < Node< vector<int> > > nn1{n1}; copies the node n1 because of data(d) where d=n1 and shows "Copied Node obj"
    • After that, executes the rest of the constructor where it shows the "create Node obj".

-- Edit:

I am with @underscore_d about using pointers instead references and you will avoid much problems :)

(@underscore_d, please don't delete your comments, they are explanatory)

-- Edit 2:

If I am not wrong, about your question regarding to:

error: binding 'const std::vector<int>' to reference of type 'std::vector<int>&' discards qualifiers

What it means is: "You have a variable const blahblah -which states that you will no modify it- and you are asigning it to a variable 'not const' so you state that maybe you will modify it. So no. I don't allow you to possibly modify it."

And especifically in your code it is because you have a non-const member in Node (notice it is a reference, key in the following):

T& data;

and in your constructors you have a const parameter and you are going to not copy, but reference it:

Node(const T & d): data(d)
Node (const Node &node): data(node.getData())

References in C++ are not pointers, but are the same object. So actually you have a "const blahblah" in one place and a "blahblah" in other. And you can convert a non-const to a const, but not the other way. You can do:

int a = 5;         // You can modify it
const int& b = a ; // and who cares if you are not going to
                   // modify 'b' (which is 'a' at the same time)

but not:

const int a = 5; // You can't modify it
int& b = a;      // and no, we are not going to let you modify it 
                 // because you would modify 'a', but 'a' is const.

Solution? Either get rid of the const paramameters, or try to make const the member of Node (but you will not be able to modify data in!).

I hope I am not wrong :)

Alfonso Nishikawa
  • 1,876
  • 1
  • 17
  • 33
  • sadly with ` T* data ` it doesn't compile with the error: `invalid initialization of reference of type 'std::vector&' from expression of type 'std::vector*' ` – Lev Jul 31 '17 at 14:03
  • A referene is really an object, so try with `data(&d)` as initialization in the constructor. And remember to change to `T& getData(){ return *data ;};`. I'm not much confident about this, since my C++ is rust :( and something tells me that holding the reference inside the Node potencially could have problems if you pass an instance allocated in the stack... – Alfonso Nishikawa Jul 31 '17 at 14:05
  • The member can be a `T&` if it only ever needs to be set once, on construction. If it needs to be changed, then yes, a reassignable `T*` is required. However, if the latter is the case, the constructor and any `set()` method should still take `T&` and convert the reference to an address internally, unless and only if `nullptr` is a valid argument. – underscore_d Jul 31 '17 at 14:09
  • This is getting messy :P – Alfonso Nishikawa Jul 31 '17 at 14:11
  • Not really, it's very basic stuff. If you want to clean up the answer/comments, then edit the answer to reflect the comments, and the comments can thus be deleted. – underscore_d Jul 31 '17 at 14:12
  • I won't delete these, but the general advice on SO is to think of comments as being transient and prone to disappearing at any time, and get the relevant info into a post asap. – underscore_d Jul 31 '17 at 14:16
  • See [Should I prefer pointers or references in member data?](https://stackoverflow.com/questions/892133/should-i-prefer-pointers-or-references-in-member-data) – underscore_d Jul 31 '17 at 14:20
1

The vector is being copied because you are initializing an object of type T with a const T&. If you only want to store a reference to T you should declare the member T& or const T& instead of T. The same thing happens for Node where you pass in node into the constructor it copies the value taken by const ref to the data member of type T which is not a reference type.

Andreas Loanjoe
  • 2,205
  • 10
  • 26