Vector is not "saving values" C++

Question

When reading from vector I put values in I got zero size. I have:

class Graph {
public:
    vector<Vertex> vertices;
};

class Vertex {
public:
    vector<int> adjacentVertices;
};

In my load method then:

int vertices, edges;
cin >> vertices >> edges;
Graph mainGraph;
mainGraph.vertices.reserve(static_cast<unsigned int>(vertices));
int tmp1, tmp2;
for (int i = 0; i < edges; i++) {
    cin >> tmp1 >> tmp2;
    mainGraph.vertices[tmp1].adjacentVertices.push_back(tmp2);
    cout << mainGraph.vertices[tmp1].adjacentVertices.size(); //PRINTS NUMBERS -> SEEMS OKAY
}

cout << mainGraph.vertices.size(); //IS ZERO???

for(const Vertex &v : mainGraph.vertices){ //CRASHES
    cout << v.adjacentVertices.size();
}

I bet this is very stupid but what am I missing? I read the vector will self construct itself upon use if no special constructor is needed.

Possible duplicate of [std::vector::resize() vs. std::vector::reserve()](https://stackoverflow.com/questions/13029299/stdvectorresize-vs-stdvectorreserve) — UnholySheep, Oct 24 '18 at 14:03
@UnholySheep you are sort of right but I did connect the resize/reserve since I did not know what the problem was and I think that if anyone in the future would google with same problem might find this useful. — eXPRESS, Oct 24 '18 at 14:08
@StoryTeller It is minimal in sense, that it provides isolated problem, I did not copy the whole load method/program ofc. But everything I put in here I thought it could be relevant to the problem. Complete, explained above. Verifiable, I do describe what is happening in quite clear way and if anyone would want to test this they can copy the code and run it themselves in no time. I think it is about ratio (since minimal and complete for example goes opposite directions a bit) and I believe the ratio is right on this one. — eXPRESS, Oct 24 '18 at 14:17
No, it's not. We do not know what `vertices` is, for instance in that static cast, for one. There's no copy and pasting the code sample on our end to check it. Read the linked page instead of interpreting the text of [mcve] yourself. — StoryTeller - Unslander Monica, Oct 24 '18 at 14:21
@StoryTeller I went through it quite fast, I admit :P, so I went through it again and the only problem I see is what you pointed out now, that i missed declaring what vertices in static_cast is, which I apologize for but I feel that you could have just pointed that out (if it was unclear) instead of directing me to page which actually could not help me to realize I forgot to put it in the code. — eXPRESS, Oct 24 '18 at 14:28

gsamaras · Accepted Answer · 2018-10-24T14:31:08.417

8

Change this:

mainGraph.vertices.reserve(static_cast<unsigned int>(vertices));

to this:

mainGraph.vertices.resize(static_cast<unsigned int>(vertices));

since with reserve() the cells of the vector are not created, which can be done though with resize().

As a result, in your code, you invoke Undefined Behavior (UB), when you do mainGraph.vertices[tmp1], since you attempt to access an object that has not been constructed!

edited Oct 24 '18 at 14:31

answered Oct 24 '18 at 14:01

gsamaras

71,951
46
188
305

More precisely, you attempt to use an object that has not been constructed and is thus not alive (although the memory _has_ been set aside already). – Max Langhof Oct 24 '18 at 14:27
Yes @MaxLanghof, let's get precise, answer updated, thanks! – gsamaras Oct 24 '18 at 14:31

score 7 · Answer 2 · answered Oct 24 '18 at 14:02

reserve does not change the size of the vector or construct items. It only allocates memory that will be reserved for when you do add elements.

Your call to mainGraph.vertices[tmp1] is undefined as it is accessing raw memory.

Instead of reserve, use resize to both allocate and construct items.

score 0 · Answer 3 · answered Oct 24 '18 at 15:42

0

That happened because you're trying to manage the size of your vectors manually. Let the STL do all the work dynamically:

Graph mainGraph;
int edge;

while (cin >> edge)
{
    Vertex newVertex;
    newVertex.adjacentVertices.push_back(edge);
    mainGraph.vertices.push_back(std::move(newVertex));
}

for(const Vertex &v : mainGraph.vertices)
    cout << v.adjacentVertices.size();

https://ideone.com/BQq01W

answered Oct 24 '18 at 15:42

Sceptical Jule

889
1
8
26

I believe that would be inefficient (in my case anyway since I am working with big data) because the initial size of vector is not the final one and it would resize couple of times while moving data in memory. The impact might be small while making mistake somewhere in algo (list vs hashmap) can cost me way more but still if there is way to optimize, why not. :D – eXPRESS Oct 24 '18 at 16:07
Well, you didn't say anything about big data. You described the problem and I wrote one possible solution based on information you gave. If you want a more suitable solution, then you should provide more details about the problem. By the way: ´std::vector::resize()´ of whatever you save your data in prevents repetitive resizing of the container. – Sceptical Jule Oct 24 '18 at 16:59
1

Yes, that is true, I am not complaining. :D Your solution is of course right too. But in the matter of efficency I think it is always better to do resizing if you know the size in advance which was implied from my example. Anyway this can be helpful to other people in future, so thank you for your answer. :) – eXPRESS Oct 24 '18 at 17:36
@eXPRESS • if you know ahead of time how big your big data is, you can call `reserve` on the vector to pre-size the vector to the exact size of your big data. Otherwise, if you don't know ahead of time how big your big data is, then just let vector do its thing. – Eljay Oct 25 '18 at 13:55
@Eljay I know :), that is kind of what my original question and answers to that were about (respectively I though I also assigned that space to vector directly, that was the problem). – eXPRESS Oct 26 '18 at 17:19

Vector is not "saving values" C++

3 Answers3