[golergka]

I don't feel that I understand purpose of this function yet.

Does this function return data point that it creates itself, or does it retrieve these nodes from somewhere else? If it's the first option, who's in charge of deleting them? If it's a second option, who's in charge of counting references to these objects? Also, if it's a second option, and this data points are retrieved from a certain storage, are you absolutely sure that whoever gets this data should be able to modify them?

[SamReidHughes]

> Does this function return data point that it creates itself, or does it retrieve these nodes from somewhere else?

It returns pointers to the nodes in the tree. You know this because the caller isn't supposed to (or allowed to) delete them, because if the caller was responsible for that, they'd be wrapped in smart pointers.

> If it's a second option, who's in charge of counting references to these objects?

The pointers in the return value are valid as long as their particular descendant of the function parameter isn't deleted. There's no need for reference counts, and if they're present at all, they aren't updated by this function, since the caller isn't responsible for decrementing them. If the caller was responsible, the return value would be a vector of smart pointers.

> Also, if it's a second option, and this data points are retrieved from a certain storage, are you absolutely sure that whoever gets this data should be able to modify them?

If they have a const node *, they shouldn't. If they don't, they should.

[golergka]

Thanks! Your first two answers would be obvious if we assume that we're working with modern codebase written by adequate developers, but that's a very strong assumption to make, so I wanted to be specific.

And your final answer is, well, exactly the point I was trying to steer the conversation to. In other words, if you declare both methods, it means that you're doing something wrong: you're giving away data mutably and immutably at the same time, and it just doesn't make sense.

[SamReidHughes]

You aren't.

If you have a non-const node, you can access its children non-constly _or_ constly (which you could do anyway, since you could make your reference to the node be const at any time). If you have a const node, you can only access its children constly.

This makes perfect sense.

[golergka]

My bad: didn't read the code carefully, thought that these were methods.

[SamReidHughes]

> if we assume that we're working with modern codebase written by adequate developers

Yeah, come to think of it if there's a chance this function came from a pre-C++11 codebase I wouldn't be so sure that the caller wasn't supposed to delete those nodes. Well, it'd be an odd thing for this particular function, I guess, but it's something I've seen or might have done, in general. In that case maybe I'd have had the function take an output parameter instead, in order to give it a scary variable name.