Hi guys,

I have this code, and I want to try to explain myself about it.

I decided to post here cause I'm not sure if I am right, and I need your help to convince me. Here goes:

const NodeList& const getChildren() const { return _children; }

How I read it:
- getChildren() will not allow changes of the instance including its members.

- It returns a constant reference of a constant NodeList (NodeList is std::vector<Node>), which means nobody should be able to modify the return value nor the reference.

My goal on this code is, I want to return a private _children where everyone can only modify its values, but not the _children itself.

Am I doing this right? if yes, is there something else in detail can be explained here? if no, what do I miss?

My goal on this code is, I want to return a private _children where everyone can only modify its values, but not the _children itself.

My suggestion is that instead of having a method that returns the collection of children itself, have a method that takes a child's index and returns a non-const reference (or maybe a pointer so that you can return nullptr if you pass in an invalid index) to that child. In this way, the clients of the class will never see the container at all. Of course, that on its own may not suit your needs if you need to iterate through the container; in which case, you might expose begin() and end() methods that delegate to those methods on _children.

const NodeList& const   [...]

which means nobody should be able to modify the return value nor the reference.

But you can't (normally) modify the reference anyway, so the second const is redundant.

With pointers, the second const actually does something... but isn't used very much. Why prevent a pointer from later pointing at something else? In almost every situation, that's not needed, and doesn't add much safety benefit. The few times I've actually needed the pointer itself to be const is fairly rare, unless you apply it as a general coding rule to, for example, every function parameter.

Making the variable be const that the reference/pointer points at, on the other hand, is much more commonly useful in my projects. By default, most of my pointers and references apply constness to their targets.

Alright folks! gonna use const Node* getChild(int index) const for now instead of getChildren() to get the whole thing. I literally have no use of it for iteration whatsoever outside itself anyway.

Now I also changed the vector to contain it with Node*. Just need to control these on my own cause there will be hundreds of them scattered around and I don't want it popping in and out from the stack (I wonder if I'm right). But I guess that's a different topic. Anyway thanks!

Alright folks! gonna use const Node* getChild(int index) const for now instead of getChildren() to get the whole thing. I literally have no use of it for iteration whatsoever outside itself anyway.

That isn't going to get you what you want. This returns a pointer to a const Node. The pointer will be mutable, but the Node itself won't be. You won't be able to modify the returned Node as you desire without removing the constness. const-correctness also won't allow this to be a const method in this case. If you want to modify something in an object through a method, that method must be non-const.

This is why I suggested returning a non-const reference or pointer above. ;)

Do you really need a vector of pointers? What do you mean by, "popping in and out from the stack"?

Alright folks! gonna use const Node* getChild(int index) const for now instead of getChildren() to get the whole thing.

Might I suggest what you really want is Node const& operator[](int index) const?

It means you can not modify the value held by your object, but you can copy it if you need to.

If you really want callers to be able to modify data aggregated in your object, you could try using object-oriented architecture and instead have your object modifiy its own data.

Alright folks! gonna use const Node* getChild(int index) const for now instead of getChildren() to get the whole thing. I literally have no use of it for iteration whatsoever outside itself anyway.

As Oberon_Command said, that means you won't be able to edit the child.

Normally what is done in these situations, is to provide two functions:

const Node* getChild(int index) const   //Called when the class is const (as accessed through whatever variable you are using to call the function)
      Node* getChild(int index)         //Called when the class isn't const

That way, when you do something like this:

MyClass myClass;

Node *node = myClass.getChild(12); //Calls the non-const version of the function, which returns a non-const node.
if(node) node->setValue("meow");   //And we can read or write to non-const nodes.

...you can change the node's values.

Same thing if you are using a non-const reference or pointer:

MyClass &myClassRef = myClass;
Node *node = myClassRef.getChild(12);
if(node) node->setValue("meow");

But when you are trying to access the function through a const variable or const reference, it prevents writing:

const MyClass &constRef = myClass;
const Node *node = constRef.getChild(12); //Calls the const version of the function, which returns a const node pointer.
if(node)
{
    node->setValue("meow"); //Properly fails to compile, since we don't want to allow editing of const classes.
    node->getValue();       //We can still read from the class though.
}

And if you do need iteration, you can just create begin() / end() functions in the class yourself, by exposing the internal container's begin() / end().

If you're using a modern C++14 compiler, it's as easy as:

class MyClass
{
public:
    auto begin()       { return children.begin(); }
    auto begin() const { return children.begin(); } //We want const and non-const iteration to be available.
    auto end()         { return children.end();   }
    auto end() const   { return children.end();   }
};

Note: If you are using an older compiler (C++11 or C++03), it's still easy to do, it just requires a smidgen more boilerplate code.

Yeah I've corrected the const. Tested it before and it didn't work so I removed them. Dang, I'm super noob. :P

At least It's good for me to know how these const works on the code I provided in the end.

Anyway Oberon gave me a good suggestion on returning the child right away in a function so I guess the case changes. It's definitely fine if only the Node itself being modified.

And I guess I'm fine without exposing the begin and end yet, but thanks for the details.

Do you really need a vector of pointers? What do you mean by, "popping in and out from the stack"?

Hmm, I was thinking that I'd rather have the child nodes in the heap instead of the stack so I can control them manually? I don't want them to be destroyed immediately unless I tell them so, which I guess that's what will happen if I put in the stack and they went out of scope or erased/removed? and there will hundreds of nodes allocated, and I don't want to mess with the stack memory for this one? Please correct this lost soul if he's mistaken. :wacko:

Might I suggest what you really want is Node const& operator[](int index) const?

It means you can not modify the value held by your object, but you can copy it if you need to.

If you really want callers to be able to modify data aggregated in your object, you could try using object-oriented architecture and instead have your object modifiy its own data.

Hmm, good suggestion, I'll check it out. :D

Do you really need a vector of pointers? What do you mean by, "popping in and out from the stack"?

Hmm, I was thinking that I'd rather have the child nodes in the heap instead of the stack so I can control them manually? I don't want them to be destroyed immediately unless I tell them so, which I guess that's what will happen if I put in the stack and they went out of scope or erased/removed? and there will hundreds of nodes allocated, and I don't want to mess with the stack memory for this one? Please correct this lost soul if he's mistaken. :wacko:

It sounds like you're getting ahead of yourself a bit. Vector allocation is already on the heap, but I don't think this is a heap/stack issue. It sounds like you want to do something unusual with lifetime management. If the 'node' is a POD then just use vector without pointers. The indirection you're introducing here is going to make iterating the vector very costly for no reason. It sounds like these objects have destructors though? I kind of want to know what's going on here, but even in that case you'd probably be better off doing manual mark and sweep if you need delayed destruction (why?).