tags:

views:

256

answers:

4

I have a class header file called Grid.h that contains the following 2 private data object:

vector<int> column;
vector<vector<int>> row;

And a public method whose prototype in Grid.h is such:

int getElement (unsigned int& col, unsigned int& row);

The definition of above mentioned function is defined as such in Grid.cpp:

int getElement (unsigned int& col, unsigned int& row)
{
    return row[row][col] ;
}

When I run the program, I get this error:

error C2109: subscript requires array or pointer type

Whats going wrong?

+13  A: 

In the line return row[row][col]; the first row is the int&, not the vector.

The variable declared in the inner scope is shadowing the variable in the outer scope, so the compiler is trying to index an int rather than a vector, which it obviously can't do.

You should fix your variable names so that they don't conflict.

EDIT: Also, while the error that you're getting indicates that the compiler is finding the wrong row variable, as A. Levy points out, you also have a problem with the declaration of your vector, so even if you fix the variable names, if you have indeed declared the vector as shown here, it won't compile. Nested templates need spaces between the > symbols, otherwise the compiler will read >> as a right-shift operator rather than part of a template declaration. It needs to be

std::vector<std::vector<int> > row;

or

std::vector< std::vector<int> > row;

In addition, as you're doing this in a header file, you're going to need to tack the std:: tag on the front of anything from the std namespace - such as vector. If it were in a cpp file, then you could use using namespace std; but that would be very bad to do in a header file (since it would pollute the global namespace). Without the std:: tag or the using statement, the compiler won't recognize vector.

Jonathan M Davis
if i change the name of the vectors to horizontal and vertical, i get undeclared identifier errors for each of them
xbonez
Alternatively you can just do "this->row[row][col]" and it should work.
Noah Roberts
+3  A: 

This is probably not the index problem, but you also need a space between the nested angle brackets in your vector of vectors type declaration. C++ compilers have a hard time telling the difference between nested template types and the right bit shift operator.

Example:

vector<vector<int> >  vec2d;        // Good.

vector<vector<int>>   anotherVec2d; // Bad!

vector< vector<int> > yetAgain;     // Best IMHO. 
                                    // Keeps the white space balanced.
A. Levy
This isn't true. Compilers ignore spaces (given that they're not contained in a string) anyway, which means that spaces don't help or hurt code, except for 'human-readability'.
KevenK
@KevenK, a space between the two symbols prevents the compiler from treating them as a single operator. I wish I could downvote your comment.
Mark Ransom
Space very much does matter in this case. The parser uses maximal munch to parse, so it grabs the largest token that it can. That means that in `vector<vector<int>>` it will parse `>>` as a shift operator rather than part of a template declaration. You need the extra space to indicate to the compiler that you're declaring a template rather than an expression with a shift operator in it.
Jonathan M Davis
This has been changed now. If you're using a current compiler you can probably write "vector<vector<int>>" and get what you expect. Not standard yet but VS2010, g++, and almost certainly others are supporting it now.
Noah Roberts
g++/gcc 4.5.0 (I just tried it) will fail to compile without the extra space. There may be a flag to tell it to use features from the upcoming standard, but without any extra compiler flags, you need the space or it will fail to compile. Now, it tells you explicitly what you did wrong instead of complaining about the shift operator, but it still fails to compile.
Jonathan M Davis
Standard C++ currently requires the space in the middle of >>, although some compilers may allow it. C++0x will make the spaceless >> legal.
Jive Dadson
@Mark/Jonathan: My apologies if my comment was not guaranteed to be correct on every compiler. I stand somewhat corrected, although truly that sounds like a shortcoming in some compilers. I can't say that I've used too many different compilers personally, but I know it's worked with Microsoft C++ compilers since at least VC++ 2005 (possibly earlier).
KevenK
The type definition "vector<vector<int>>" is not legal c++ under the current standard. It will be legal under c++0x, according to the draft proposal.
Jive Dadson
So, really, it's a deficiency in the language, not the compiler. Some compiler writers chose to break the standard in order to get around that deficiency (which is handy if that's the only compiler you use but otherwise hurts portability and causes problems), while others chose to follow the standard. Fortunately, the next standard fixes this deficiency in the language so that it will no longer be an issue. I really wouldn't say that it's a deficiency in the compilers though, since that's the way the current standard is.
Jonathan M Davis
+1  A: 

It appears to me (although you may need to verify this on your own, I don't feel like writing up a test application) that the problem is coming from the fact that your parameter contains a named row and your class has an inner variable row and there is a naming conflict.

You may need to qualify which row you're using. Consider:

return Grid::row[row][col];
KevenK
Or return this->row[row][col]
a1ex07
I renamed row and column in the private: section to horizontal and vertical. I now get the error'horizontal: undeclared identifier' and 'vertical: undeclared identifier'
xbonez
@xbonez: Why do you have two vectors? I thought you wanted one vector of vectors? (Giving a 2D vector.)
GMan
I wish I could upvote this in good conscience, since it's technically correct. It's just the wrong solution to the problem.
Mark Ransom
@GMan: I made another vector 'vector<int> column, so when I want to add a vector to row, I can do row.push_back(column). When i use push back on row, I have to pass a vector as an argument, don't I?
xbonez
+1  A: 

I think you want something like this... (although I cannot imagine why :-))

#include <vector>
#include <iostream>

using namespace std;

typedef vector<int> row;
typedef vector<row> matrix;

matrix mat(2,2);

int getElement (unsigned int ri, unsigned int ci)
{
    return mat[ri][ci] ;
}

int main() {

    mat[1][0] = 1234;
    cout << getElement(1,0) << endl;

    return 0;
}
Jive Dadson
aahhh!! yes, that seems a lot better way to do it. I didn't think of it. This way I don't need to nest vectors. Thanks a lot. I'm gonna use it this way.
xbonez