0

The error is : "member matrix::rows is not a type name" and "member matrix::init is not a type name". If it compiles successfully I shall have a matrix rows = ROWS and columns = COLS. What am i doing wrong here:

#include <vector>
class matrix
{
    int ROWS{}, COLS{}, init{-1};
    std::vector<std::vector<int>> table(ROWS, std::vector<int>(COLS, init));
};
John Zwinck
  • 239,568
  • 38
  • 324
  • 436

3 Answers3

2

As John Zwinck points out, you need curly brackets. Why? Because, otherwise, the compiler takes

std::vector<std::vector<int>> table(ROWS, std::vector<int>(COLS, init));

as the definition of a member function, called table, returning a std::vector<std::vector<int>> and taking a first (unnamed) parameter of type ROWS. At which point the compiler goes "Ugh, I don't know the type ROWS, let me throw a compilation error".

You can google Most Vexing Parse. https://en.wikipedia.org/wiki/Most_vexing_parse

Jeffrey
  • 11,063
  • 1
  • 21
  • 42
  • But if i use curly brackets instead, then the size of the matrix is no longer ROWS*COLS, it is something else. https://github.com/whysosaxy/learn/blob/main/test.cpp here it shows that the sizeof of matrix is 24!! HOW? – user222061 Jun 26 '21 at 08:22
  • Sizeof tells you the size of the outermost vector object. This vector allocates dynamic memory for the innermost vector. 24 is the begin pointer, the reserved size and the used size. 3x 64 bits. – Jeffrey Jun 26 '21 at 14:22
  • This time I tried using the size function and it checks. But there is still a problem. When I try to print the elements of the matrix something bizzare happens. I am able to print all 50 rows but only 2 columns, where all the elements of the first column is 50 (should have been 0). Where are the other elements of the matrix? Please see the changed code here : https://github.com/whysosaxy/learn/blob/main/test.cpp – user222061 Jun 27 '21 at 08:48
  • The wrong constructor is used. It creates vectors with 50 and 0 as elements, instead of 50 times 0. This would make an excellent new question (but check for duplicates first). – Jeffrey Jun 27 '21 at 15:48
1

You can write it like this:

class matrix
{
    static constexpr int ROWS{}; // TODO: change to non-zero
    static constexpr int COLS{}; // TODO: change to non-zero
    static constexpr int init{-1};
    std::vector<std::vector<int>> table{ROWS, std::vector<int>(COLS, init)};
};
John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • 1
    Maybe mention that this is yet another case of the most vexing parse? – Jeffrey Jun 26 '21 at 02:13
  • But if i use curly brackets instead, then the size of the matrix is no longer ROWS*COLS, it is something else. https://github.com/whysosaxy/learn/blob/main/test.cpp here it shows that the sizeof of matrix is 24!! HOW? – user222061 Jun 26 '21 at 08:24
  • @user222061: That's because sizeof() does not tell you the *total* memory usage of an object. See https://stackoverflow.com/questions/34024805/c-sizeof-vector-is-24 – John Zwinck Jun 26 '21 at 08:42
  • This time I tried using the size function and it checks. But there is still a problem. When I try to print the elements of the matrix something bizzare happens. I am able to print all 50 rows but only 2 columns, where all the elements of the first column is 50 (should have been 0). Where are the other elements of the matrix? Please see the changed code here : https://github.com/whysosaxy/learn/blob/main/test.cpp – user222061 Jun 27 '21 at 08:49
  • @user222061 Sorry I had a bug in my answer. `{COLS, init}` should have been `(COLS, init)` (and now it is). That's because `std::vector{COLS, init}` calls the `initializer_list` vector constructor and not the `(count, value)` constructor. I've edited my answer now, and ran it to confirm it makes a 50x50 matrix now. – John Zwinck Jun 27 '21 at 09:09
0

I don't understand the usage of the vector of vectors for this case. For such a case, I would propose to use a plain array.

template<int N, int M, typename D> struct Matrix {
public:
    D get(int i, int j) {
        return _data[index(i,j)];
    }
    void set(int i, int j, D v) {
        _data[index(i,j)] = v;
    }

private:
    int index(int i, int j){
        return i*N + j;
    }
    D _data[N*M];
};

So you don't really need std::vector. Just add some checks and you are good.

  • Although this accomplishes what OP was trying to accomplish, it doesn't answer the question of what they were doing wrong – Human-Compiler Jun 26 '21 at 04:40
  • As I mentioned in my response - I don't see the point of the complication of simple things. Also, my approach IMO will show better performance – Sergey Strashko Jun 26 '21 at 04:44