1

So I declared the following map in C++:

typedef vector<Vector2D> stackPoints; 
typedef map<int, stackPoints> mapPoints; 
mapPoints drawingPoints;

After adding values to it, I want to output all the elements in it, but the vectors at the different key positions are not of the same size:

I am using the following two for-loops that don't work. Sometimes the program crashes at runtime and gives me the out of range vector error.

for (int j = 0; j < drawingPoints.size(); j++)
{
    for (int i = 0; i < drawingPoints[j].size(); i++)
    {
        cout << "(...)" << endl
    }
}

It seems to me that the inner for-loop has the be went through a constant amount of times, as if the following scenario wasn't possible:

1) The first vector has a size of 1, so the inner for-loop will be executed once.

2) Then the second vector of the map has a size of 5 and now I want the for-loop to be went through 5 times, but this seems to not work.

** Edit **

I am using the integer key as a counter, so I increment it by 1 when I add another pair.

huzzm
  • 489
  • 9
  • 24
  • 1
    `drawingPoints[j]` gets the entry with key `j`, not the `j`-th entry! – aschepler Dec 17 '15 at 21:48
  • Yeah, that is what I want, isn't it? Depending on the size of the vector at drawingPoints[ j ] - which changes! - I want the inner for-loop to be executed just as often. – huzzm Dec 17 '15 at 21:51
  • For your edit, if the int is just an index indicating the number of entries, why not just use a `std::vector`. Then you can do exactly what you have done. – Fantastic Mr Fox Dec 17 '15 at 23:06
  • Yeah, you are totally right, my bad ^^ – huzzm Dec 18 '15 at 23:23

3 Answers3

2
for (int j = 0; j < drawingPoints.size(); j++)
{
    // This is wrong.
    // j is not necessarily a valid key in the map.
    for (int i = 0; i < drawingPoints[j].size(); i++)
    {
        cout << "(...)" << endl
    }
}

Use iterators to avoid such issues.

mapPoints::iterator map_begin = drawingPoints.begin();
mapPoints::iterator map_end = drawingPoints.end();
for ( ; map_begin != map_end; ++map_iter )
{
   stackPoints::iterator v_iter = map_iter->second.begin();
   stackPoints::iterator v_end = map_iter->second.end();
   for ( ; v_iter != v_end; ++v_iter )
   {
      cout << "(...)" << endl
   }
}

If you are using a C++11 compiler, you can use:

for ( auto& map_item : mapPoints )
{
   for ( auto& v_item : map_item.second )
   {
      cout << "(...)" << endl
   }
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
2

For multiple reasons you should be using an iterator.

  1. Your map may not contain every element from 0 to drawingPoints.size() and for each element that it doesnt contain when you loop over it you will create it.
  2. Your highest entry may be greater then drawingPoints.size() in which case you will never reach it.

Lets look at outer most loop, if you want to define an iterator for it you should do the following:

for (mapPoints::Iterator it = drawingPoints.begin(); it != drawingPoints.end(); it++) {

This creates an iterator which you can look at (by doing it-> or *it).

More information can be found here.

Fantastic Mr Fox
  • 32,495
  • 27
  • 95
  • 175
1

The problem is iterating the map: the fact that you can do drawingPoints[j] is entirely by accident - the type of your map's key just happens to be int, so j works fine as its index. However, not all js are going to have anything in the map, so the code does not work.

Here is how you iterate over your map:

for(auto& kvp : drawingPoints) {
    cout << "Key " << kvp.first << " has the following values:" << endl;
    for (auto n : kvp.second) {
        cout << n << " ";
    }
    cout << endl;
}

The example above requires C++11. If you use an older compiler, use this Q&A to go over a map using iterators.

Community
  • 1
  • 1
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • I am using the integer key as a counter, so I increment it by 1 when I add another pair. – huzzm Dec 17 '15 at 21:55
  • @huzzm exactly, it adds another pair, unless the key happens to be there, in which case you iterate over its corresponding vector. – Sergey Kalinichenko Dec 17 '15 at 21:55
  • @huzzm Yes, it would work. However, if the key is always there when you iterate sequentially, then you are better off with a vector of vectors, because vectors are generally faster and take less memory than a map. – Sergey Kalinichenko Dec 17 '15 at 22:03