2

I am somehow aware of returning by reference by this got me confused.

I have a function which returns a reference to a public field of class BoardNode, std::vector<BoardNode> _neighboursVector.

I also have a class Board which holds a std::vector<BoardNode>.

my member function goes like this:

const std::vector<BoardNode>& 
Board::getNeighboursVector(unsigned int x, unsigned int y) const
{
    BoardNode node = this->getBoardNode(x, y);
    //...
    node._neighboursVector.push_back(...);
    //...
    return node._neighboursVector;
}

While debugging on return line I get the correct values in the vector but outside of this function I get empty vector. Why ?

std::vector<BoardNode> v = b.getNeighboursVector(5,5);

EDIT

getBoardNode definitions

const BoardNode & Board::getBoardNode(unsigned int rowIdx, unsigned int colIdx) const
{
//...
}

BoardNode & Board::getBoardNode(unsigned int rowIdx, unsigned int colIdx)
{
//...
}
Patryk
  • 22,602
  • 44
  • 128
  • 244

2 Answers2

6

node is a local object. And by extension, node._neighborsVector is also a local object. As a local object, it gets destroyed at the end of the function. So you are returning a reference to a destroyed object. That's undefined behavior.

Benjamin Lindley
  • 101,917
  • 9
  • 204
  • 274
  • Adding to that, http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope : There is a massively upvoted answer which describes very nicely about scope and access. – Arvind Nov 23 '12 at 14:36
2

node is created on the stack (local to your function) and thus deleted at the end of the function. Since the reference you return is a field of node it is also deleted. Therefore you return a reference to a deleted object.

You should either return by value (in this case correctly implement the copy constructor - here for std::vector it's ok) or by pointer (created by new and don't forget to delete when you're done with the returned object).

Antoine
  • 13,494
  • 6
  • 40
  • 52
  • 1
    You shouldn't return rvalue references to local object either. – avakar Nov 23 '12 at 14:27
  • Can't vote for this answer since it suggests returning a pointer. – john Nov 23 '12 at 14:30
  • @BenjaminLindley you're right - it's been some time since I played with move. – Antoine Nov 23 '12 at 14:43
  • @john: that's just absurd. C/C++ has pointers, it's a language feature. If the vector is long to build and has millions of entries it's really better to return pointer/smart-pointer instead of copied object. – Antoine Nov 23 '12 at 14:46
  • maybe I wasn't clear enough, obviously the pointed object has to be allocated with `new` – Antoine Nov 23 '12 at 14:48
  • Well smart pointer is another thing. The implication of the answer is a regular pointer since you mention delete. – john Nov 23 '12 at 14:49
  • Smart pointers are more convenient and secure. That doesn't make regular pointers invalid. – Antoine Nov 23 '12 at 14:51