tags:

views:

150

answers:

4

I'm currently trying to print out a history of movements for players in a game I am working on. At the end of each round every player has moved some amount in the positive or negative direction and this gets recorded as an int in the movement vector. Eventually I'm wanting to plot the directions moved vs time for each player but I'm having trouble extracting the data out of the 2d vector.

So the first thing I tried was to just iterate and print all the elements, however this doesn't compile:

void output_movement(const std::vector< std::vector<int> > & movement){

    std::vector< std::vector<int> >::iterator row;
    std::vector<int>::iterator col;
    for (row = movement.begin(); row != movement.end(); ++row) {
         for (col = row->begin(); col != row->end(); ++col) {
            std::cout << **col;
         }
    }

}

The compiler gives this error message which I don't really understand:

hg_competition.cpp:45: error: no match for ‘operator=’ in ‘row = ((const std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >*)money_movement)->std::vector<_Tp, _Alloc>::begin [with _Tp = std::vector<int, std::allocator<int> >, _Alloc = std::allocator<std::vector<int, std::allocator<int> > >]()’
/usr/include/c++/4.4/bits/stl_iterator.h:669: note: candidates are: __gnu_cxx::__normal_iterator<std::vector<int, std::allocator<int> >*, std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > >& __gnu_cxx::__normal_iterator<std::vector<int, std::allocator<int> >*, std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > >::operator=(const __gnu_cxx::__normal_iterator<std::vector<int, std::allocator<int> >*, std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > >&)

Any help is greatly appreciated!

+4  A: 

const objects return const_iterators, so simply replace iterator by const_iterator everywhere. This also prevents unwanted modifications of the vectors.

This is the combination of Sam's and Mathieu's suggestions:

#include <ostream>
#include <vector>

typedef std::vector<int> Vector;
typedef std::vector<Vector> DoubleVector;


template<typename Char, typename Traits>
std::basic_ostream<Char, Traits>&
operator<<(std::basic_ostream<Char, Traits>& stream,
           const DoubleVector& movement) {
    for (DoubleVector::const_iterator row = movement.begin(); row != movement.end(); ++row) {
         for (Vector::const_iterator col = row->begin(); col != row->end(); ++col) {
            stream << *col;
         }
    }
return stream;
}
Philipp
+9  A: 

The 2D vector is declared const, so you need to use const_iterator instead of iterator.

You also shouldn't doubly dereference col. It is an iterator, so you only need to dereference once.

void output_movement(const std::vector< std::vector<int> > & movement){ 

    std::vector< std::vector<int> >::const_iterator row; 
    std::vector<int>::const_iterator col; 
    for (row = movement.begin(); row != movement.end(); ++row) { 
         for (col = row->begin(); col != row->end(); ++col) { 
            std::cout << *col; 
         } 
    } 

} 
Peter Alexander
+9  A: 

You need to use a const_iterator if the vector is a const reference. Also, to output col you only need to dereference it once.

void output_movement(const std::vector< std::vector<int> > & movement){

    std::vector< std::vector<int> >::const_iterator row;
    std::vector<int>::const_iterator col;
    for (row = movement.begin(); row != movement.end(); ++row) {
         for (col = row->begin(); col != row->end(); ++col) {
            std::cout << *col;
         }
    }
}

Edit: using typedefs will make your code more readable

typedef std::vector<int> Vector;
typedef std::vector<Vector> DoubleVector;

void output_movement(
    const DoubleVector& movement
)
{
    for (DoubleVector::const_iterator row = movement.begin(); row != movement.end(); ++row) {
         for (Vector::const_iterator col = row->begin(); col != row->end(); ++col) {
            std::cout << *col;
         }
         std::cout << std::endl;
    }
}
Sam Miller
I would argue that if you're going to rewrite the code, it could be worthwhile to also properly put `row` and `col` declarations within the `for` loop for proper scoping.
Matthieu M.
Also it would perhaps be a bit more C++-like if `output_movement` were called `operator<<`.
Philipp
@Matthieu I've updated my answer to include your suggestion.
Sam Miller
@Philipp: I am not sure. Overloading operator is always tricky, especially when there could be several possible displays.
Matthieu M.
A: 

Omg, anything is better than that mess of for loops. Here some alternatives. Choose whichever you like.

typedef vector<int> VI;
typedef vector<VI> VVI;


namespace std {
    ostream& operator<<(ostream& o, const VI& v) {
        copy (v.begin(), v.end(), ostream_iterator<int>(cout));
        return o;
    }
}
void output_movement (const VVI& m) {
    copy (m.begin (), m.end (), ostream_iterator<const VI&>(cout));
}

or,

void output_movement (const VVI & m) {
    for_each (m.begin(), m.end(), [](const VI& v){ 
                for_each (v.begin(), v.end(), [](int i){ cout << i; });
                });
}

or, my personal preference (boost/foreach.hpp),

void output_movement (const VVI & m) {
    foreach (const VI& v, m)
        foreach (int i, v)
            cout << i;
}
John