1

I've been trying to write a Matrix class and I am using a main method to test it out. It isn't working... at all.

I don't understand why size (the size of allRowValues divided by the size of double) is zero!

I've been writing some debug prints but it isn't helping... I'm really, really new with C++ so ANY and all help / advice will be appreciated.

 1 #include "matrix.h"
 2 #include <iostream>
 3 #include <sstream>
 4 
 5 Matrix::Matrix(){
 6 };
 7 
 8 Matrix::Matrix(int rows, int columns, double allRowValues [] ){
 9     this->rows = rows;
10     this->columns = columns;
11     this->matrixValues = new double[rows*columns];
13     std::cout <<"ALL ROW VALUES" <<std::endl;
14     std::cout<<"*****" <<std::endl;
15     std::cout << sizeof (allRowValues) << std::endl;
16     std::cout<<"*****" <<std::endl;
17     std::cout << sizeof(allRowValues[0]) << std::endl;
18     std::cout<<"*****" <<std::endl;
19     int size = sizeof(allRowValues)/sizeof(double);
20     int numberOfValues = rows * columns;
21     int currentIndex = 0;
22     for (int i = 0; i < numberOfValues; i++){
23             std::cout<< "MATRIX CONSTRUCTOR\n";
24             std::cout<<allRowValues <<std::endl;
25             std::cout<<"-----"<<std::endl;
26             std::cout<<index << std::endl;
27             std::cout<<"-----"<<std::endl;
28             std::cout<<size << std::endl;
29             std::cout<<"-----"<<std::endl;
30             if ((allRowValues) && (currentIndex < size)){
31                 std::cout << "ARV " <<std::endl;
32                 std::cout << allRowValues[currentIndex] << std::endl;
33                 this->matrixValues[i]= allRowValues[currentIndex];
34                 std::cout << "MAT " << allRowValues[currentIndex++] << std::endl;
35             }else{
36                 std::cout << "Else\n";
37             }
38         }
39         int index=0;
40         for (int j = 0; j < rows; j++){
41             for (int i = 0; i < columns; i++){
42                 std::cout << this->matrixValues[index++];
43             }
44             std::cout<<std::endl;
45         }
46     };
47 
48     Matrix::Matrix(double* rowValues){
49         int sizeRows = sizeof(rowValues)/sizeof(double);
50         //TODO: throw error for all rows must be the same length
51         this->rows = sizeRows;
52         int sizeColumns = sizeof(rowValues[0])/sizeof(double);
53         this->columns = sizeColumns;
54         this->matrixValues = rowValues;
55     };
56 
57     double Matrix::width(){
58         std::cout << "Width\n";
59         return this->columns;
60     };
61 
62     double Matrix::height(){
63         std::cout << "Height\n";
64         return this->rows;
65     };
66 
67     std::string Matrix::toString(){
68         int numberOfValues = 0;
69         std::cout<<matrixValues[numberOfValues];
70         std::string build_output;
71         std::cout<<matrixValues;
72         for (int i = 0; i < rows; i++){
73             build_output = "[";
74             std::cout << "\n";
75             for (int j = 0; j < columns; j++){
76                 std::cout << "VALUE: " <<matrixValues[numberOfValues];
77                 build_output = matrixValues[numberOfValues];
78                 numberOfValues++;
79             }
80             build_output = " ]";
81         }
82         return build_output;
83     }
84 
85     int main (){
86         double values[6] = {1, 2, 3, 4, 5, 6};
87         std::cout <<"Values: \n";
88         Matrix a = Matrix(2, 3, values);
89         std::cout << a.width() << std::endl;
90         std::cout << a.height() << std::endl;
91         std::cout << a.toString();
92         return 1;
93 }
coconut
  • 1,074
  • 4
  • 12
  • 30
  • 5
    is this homework? If not, there are lots of very good matrix libraries written for C++, which will be a lot faster and less buggy than doing it yourself. – Shep Apr 20 '12 at 16:42
  • 1
    Even if you decided to do this yourself, don't force the user to enter the data interactively. Have them put the data in a file, and just give you a file name. This will simplify your code and be *much* less error prone for the user. – Jerry Coffin Apr 20 '12 at 21:36
  • `sizeof` is a bad design. What if your compiler chooses to make int 8 bytes on 32 bits, and 16 bytes on 64 bits machine? Don't rely on `sizeof`. – CppLearner Apr 21 '12 at 03:45
  • @CppLearner what would be the problem then? `sizeof` is much more reliable than magic numbers. – R. Martinho Fernandes Apr 22 '12 at 16:49

3 Answers3

2

double allRowValues[] declares a pointer, not an array. The expression sizeof(allRowValues)/sizeof(double) then computes the ratio between the size of a pointer and the size of a double. If doubles are bigger, the result is obviously zero.

For some reason, the same mistake happens in the other constructor too (sizeof(rowValues)/sizeof(double)), but this time the argument is clearly a pointer. And then there's sizeof(rowValues[0])/sizeof(double) which is the ratio between the size one element of an array of doubles, i.e. a double, and the size of a double, which is obviously one.

It seems there's the belief that the sizeof operator can magically know the size of an array given a pointer to its first element. It can't. Or maybe there's confusion between arrays and pointers. They're not the same.

Most of the time, arrays (i.e. objects of type T[N], like double[100]) just decay to pointers to their first element (i.e. T*, like double*), losing size information in the process. This is why you can never just pass a pointer if you intend to "pass an array". You need to pass the size information in somehow.

You can explicitly pass the size as an extra argument, or pass another pointer that marks the end of the buffer (iterator style). You can also pass a reference to the array (which prevents decaying into a pointer, thus preserving the size information) and use a template to obtain the size information.

template <std::size_t N, std::size_t M>
void pass_a_2d_array_by_reference(double(&the_array)[N][M]) { // N and M are the sizes
    // do stuff
}

Now that you understand these problems, you can not-have them at all if you use an off-the-shelf solution:

  • std::vector<std::vector<double>>: it's a very good solution if you don't need contiguous storage. It's also your best shot if you want jagged arrays.
  • boost::multiarray<double, 2>: another very good solution that works just as well with arrays of even more dimensions;
  • there are many others existing solutions, for various needs. Just look around.
Community
  • 1
  • 1
R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
  • I also wrote a simple two-dimensional matrix class template that you can look over and take ideas from, in case this is just a learning exercise: http://ideone.com/gytw7 – R. Martinho Fernandes Apr 20 '12 at 16:56
  • It might be worth providing a template constructor: `template Matrix::Matrix( double (&init)[N][M] )`, to allow initialization from a C style matrix (typically `const` and `static`). – James Kanze Apr 20 '12 at 17:02
  • Yes! I was adding that as you wrote the comment :) – R. Martinho Fernandes Apr 20 '12 at 17:07
  • 1
    Concerning `std::vector >`, one of its disadvantages is that it allows jagged arrays (which you don't want in a matrix). Of course, if it's all encapsulated in a class which ensures that they aren't jagged, it's no problem (although I still prefer `std::vector` and doing manual index calculations). – James Kanze Apr 20 '12 at 17:28
0

If you insist on c-style arrays (which I don't recommend) you could also use a templated solution:

template<int size>
Matrix::Matrix(int rows, int columns, double (&allRowValues) [size] ){
...
}

Overall, I'd recommend an off the shelf, permissive open-source matrix library such as eigen.

enobayram
  • 4,650
  • 23
  • 36
-1

You should use a std::vector<double>& (or a std::array) instead of c-style array double allRowValues[]. Thus you could easily get its size with allRowValues.size().

c++ faq-lite: Why should I use container classes rather than simple arrays?

Reference:
http://en.cppreference.com/w/cpp/container/vector
http://en.cppreference.com/w/cpp/container/array

log0
  • 10,489
  • 4
  • 28
  • 62
  • Change the _should_ to _could_ and I'll take back my -1 – Shahbaz Apr 20 '12 at 16:57
  • I'd argue for _should_, although possibly as an additional constructor. There's a good argument for supporting initialization from a C style matrix, although in that case, he should get the size from the C-style matrix, using a template constructor, and not take a `double*`. – James Kanze Apr 20 '12 at 17:04
  • @Shahbaz I don't understand. What would be the advantages of a c-array over a c++ vector ? Then about c-style matrix, I also see a plus if the matrix dimensions are templates arguments. In this case there is now `std::array,n>`. – log0 Apr 20 '12 at 17:29
  • @ugo If all you need a is a simple array, why use a vector? This answer could _suggest_ a vector as an alternative, which is understandable. – Shahbaz Apr 23 '12 at 03:43
  • @Shahbaz I would stick to this http://www.parashift.com/c++-faq-lite/containers.html#faq-34.1 – log0 Apr 23 '12 at 07:05