CODEWITHSUNDEEP
c++pointersmemorydictionaryreference
wasp
wasp

Reputation: 3

std::map with pointers: wrong value assess

I am trying to save into the STL map a pointer to the Block object:

void IO::parseInput(void)
{
    map<string, Block*> blocksMap;

    //=== Create new block and write it to the vector in database ===
    Block tmpBlock (bIndex, bName, bWidth, bHeight);
    Block* tmpPointer = db.addBlock(tmpBlock); //addBlock returns pointer to the added Block

    tmpPointer->printParams(); // -- here is correct output

    blocksMap.insert ( pair<string, Block*>(bName, tmpPointer) );

    // === Test hash ===
    blocksMap.find("anyLegalStringKey")->second->printParams(); // -- wrong output
}

addBlock function in DB class:

Block* DB::addBlock (Block& newBlock)
{
    blocks.push_back(newBlock);
    Block* ptrToLast = &blocks.back();
    return ptrToLast;
}

Vector of Block's in DB class:

class DB:
{
    private:
    //=== Database ===
    vector <Block> blocks;
};

Problem: Before writing into the map I access to the object (which pointer I want to save) using pointer tmpPointer and print all its parameters. This one works correct. Then I save this pointer in map with specific key. When i try then to access the same pointer using find in map with specific key, I get a wrong output (I also print all parameters).

When I try to access to pointers in map for any existed key it could lead to four different reactions:

  1. Everything is OK and I get normal output (in my case I print all parameters)
  2. I get wrong parameters (e.g. instead of index 7 I got 21814704)
  3. Unreadable output
  4. Segmentation fault

Interesting is that for the same Block object I have always same reaction (e.g. block with name "g22i" has always unreadable output).

Vector "blocks" in db contains correct info before and after I save pointers in map.

Thanks!

Upvotes: 0

Views: 825

Answers (1)

AnT stands with Russia
AnT stands with Russia

Reputation: 320421

You are trying to use long-lived pointers to std::vector's elements. That's a recipe for disaster. All such pointers will be invalidated the very moment your vector decides to reallocate itself.

If you want to use pointers to vector elements, you have to make sure the vector never reallocates. I.e. you have to reserve enough capacity in advance to store all future elements. Most of the time it is suboptimal, defeats the purpose of using std::vector or plain impossible.

You can also use a container that never invalidates pointers to its elements, like std::list. But std::list does not support random access. You can use std::deque, which supports random access (albeit less efficiently than std::vector) and preserves the validity of element pointers as long as you don't insert anything into the middle of the sequence.

Finally, you can keep using std::vector, but make sure to store [smart] pointers to Block objects in it instead of Block objects themselves. The idea is to ensure that vector reallocation does not cause reallocation of the actual Block objects.

P.S. Another remark: when you declare a map<string, Block*> blocksMap, elements of such map have type pair<const string, Block*> (note the extra const). I.e. map<string, Block*>::value_type is actually pair<const string, Block*>.

Later you are trying to call blocksMap.insert with pair<string, Block*> argument, while blocksMap.insert actually expects pair<const string, Block*>. This compiles, but it involves an implicit conversion from your pair<string, Block*> to pair<const string, Block*> performed by std::pair's conversion constructor. This is not optimal performance-wise. Which is why a better idea might be to use map<string, Block*>::value_type instead of trying to spell the element type manually

blocksMap.insert ( map<string, Block*>::value_type(bName, tmpPointer) );

Upvotes: 4

Related Questions