0

I looked into two different method to allocate memory for the elements of a matrix

Method n.1

int** matrix = new int*[rows];
for (int i = 0; i < rows; ++i)
    matrix[i] = new int[cols];

Method n.2

int** matrix = new int*[rows];
if (rows)
{
    matrix[0] = new int[rows * cols];
    for (int i = 1; i < rows; ++i)
        matrix[i] = matrix[0] + i * cols;
}

I can figure out what Method n.1 does, but I can't figure out what exactly is supposed to do the if clause in Method n.2 (I would implement it without and it doesn't work, with the if clause, it does...)

EDIT: Here is a code showing my problem. Why does it take so long to load (~30seconds)?

http://codepad.org/uKvI8Tk3

Codepad refuses to show the output (timeout) so if you want to run it, just compile it on your own.

Also, why cout << statements are not executed once the program starts?

doplumi
  • 2,422
  • 4
  • 26
  • 40
  • 2
    What happens if rows is zero in the second case? – Roddy Feb 17 '13 at 22:23
  • I don't understand why in method 2, the first allocation isn't inside the `if` either. – Mr Lister Feb 17 '13 at 22:24
  • @MrLister - new int*[0] still effectively allocates 'something' but you cant dereference it. http://stackoverflow.com/questions/1087042/c-new-int0-will-it-allocate-memory (but, yes, it still looks a bit dubious...) – Roddy Feb 17 '13 at 22:26
  • @Roddy If row is 0, it doesn't have sense to create the matrix. – doplumi Feb 17 '13 at 22:30
  • @domenicop In method 1, `rows == 0` will lead to nothing useful, but it won't crash. In method 2, the `matrix[0] = ...` may easily crash the program. – us2012 Feb 17 '13 at 22:32
  • @Roddy and if row is 0, even the first method doesn't make sense, right? Also, I can't understand why program will compile but not run. I mean, what happens inside? – doplumi Feb 17 '13 at 22:33
  • If row is zero the second method will crash without the if. See @us2012's comment. – Roddy Feb 17 '13 at 22:36
  • So, in method #2 the program won't run even if compiled because it prevents itself from crashing? Does this kind of behaviour has a name? – doplumi Feb 17 '13 at 22:46
  • @domenicop : No, it should just crash if rows are zero. Can you explain *exactly* what behaviour you're seeing: Error messages, etc... And maybe a small self-contained compilable example. – Roddy Feb 17 '13 at 22:55
  • It _does_ start, but after a **bunch** of time. It's really strange. Now, even if I include the if clause, it starts, but this is the scenario 1. CMD opens; 2. 40 seconds pass with cmd opened completely empty; 3. First lines of output (asking for matrix dimensions) appears. The strange thing is that I declare the matrix after asking for its dimensions, so why the cout statements aren't executed as soon as I run the .exe? – doplumi Feb 18 '13 at 09:52
  • @domenicop : Time for you to post some more code, please! There's no `cout` statements in the code above. Show how you're using the above code, and how/where the matrix is defined. – Roddy Feb 18 '13 at 12:42
  • @Roddy what code do you want to see? I'm talking about the code posted before with a simple and obvious cout << "Enter rows"; cin >> rows; cout << "Enter columns"; cin >> cols; before – doplumi Feb 18 '13 at 20:54
  • @domenicop - The behaviour you describe doesn't make sense given the code posted so far, so the problem must lie elesewhere. Try making a complete runnable example on ideone.com or somewhere similar, so it can be seen and run online. – Roddy Feb 18 '13 at 22:28
  • @Roddy sorry for the wait, here is the code: http://codepad.org/uKvI8Tk3 Codepad.org reports timeout as output :S – doplumi Feb 20 '13 at 18:56
  • @domenicop : check this line *very* carefully. `for ( int col = 0; col < cols; ++cols )` – Roddy Feb 21 '13 at 00:28
  • yeah, thank you! Anyway, I fixed it, and I still cant figure out why the two output method I inserted do not result the same thing. New code: http://codepad.org/uKZRcxMp (You can see output below the code) – doplumi Feb 21 '13 at 19:33
  • @domenicop, Its because you have not allocated a single contiguous lump of memory. You allocate an array of pointers (`matrix = new int * [ rows ]`) then allocate memory memory for each row. The second output method is just printing the pointers to the memory allocated for each row : `0x8051468 0x8051498 0x80514c8` - and then it runs off the end of the allocated area, so the rest is garbage. – Roddy Feb 21 '13 at 21:15
  • @Roddy What about this? Why method 2 here doesn't work? http://codepad.org/Ipek40Xo – doplumi Feb 22 '13 at 14:39
  • @domenicopv : operator precendence of + vs. * : Try `*( *(matrix + i) + j)` – Roddy Feb 22 '13 at 15:41
  • @Roddy you wrote something I don't understand. I'll write my thinking process to reach matrix[ i ][ j ]. 1 I want to reach the location where pointers are located, so i write *matrix. 2 Once I've reached them, I want to add i*sizeof( *int ) to go to the pointers that will lead me to the first element of the _i_ th row, so I write *( *(matrix) + i ). 3 I have to add j*sizeof(int) to go to the _j_ th element of the row ( that is, matrix[ i ][ j ] ) so I write *( *(matrix) + i ) + j. Which passage is the wrong one? – doplumi Feb 24 '13 at 16:26
  • And why I obtain the same (right) output with *( *(matrix + i) + j) and **(matrix + i) + j – doplumi Feb 24 '13 at 16:33

2 Answers2

5

Method n.3: write your own Matrix class, internally using a single std::vector<int> and being clever about access by (row,col) indices.

struct Matrix
{
  explicit Matrix(unsigned int rows, unsigned int cols) : data_(rows*cols), cols_(cols) {}
  const int& operator()(unsigned int row, unsigned int col) const
  {
    return data_[row*cols_ + col];
  }
 private:
  std::vector<int> data_;
  unsigned int cols_;
};

Edit: iff the memory overhead of a vector is an issue in the last example, you can consider using a single dynamically allocated array of length rows*cols, and make sure to call delete [] on it in the destructor.

juanchopanza
  • 210,243
  • 27
  • 363
  • 452
  • Method 3 is awful. Extra memory wasted for no apparent reason. Data locality trashed. – haffax Feb 17 '13 at 22:27
  • I'm not convinced a matrix class should internally use `>`. For reasons of memory locality, it should use method 2 to allocate a continuous block. – us2012 Feb 17 '13 at 22:27
  • @haffax how much extra memory? Does it matter? – juanchopanza Feb 17 '13 at 22:29
  • @us2012 You are right, once you have your own matrix class and are free to chose the representation, it makes little sense to have a vector of vectors. – juanchopanza Feb 17 '13 at 22:30
  • @juanchopanza For something basic as a matrix the extra memory will be a sizable bit of it for small arrays. In most implementations 8 bytes per col and 8 bytes for the whole construction. For a 4x4 matrix this is 40 bytes structure memory for 64 bytes of actual data. Worse is, that the data is not available in one block, but spread over indiviual blocks per col. Which makes common matrix operations very cache unfriendly. – haffax Feb 17 '13 at 22:36
  • 1
    @haffax - a whole 40 bytes wasted ;-) The answer to "does it matter" is as always, "it depends". Depends on memory constraints, array size, and number of objects being created. Boost multi_array is more efficient, but writing the notation to create one would have me wasting serious time unless I needed it day to day. – Roddy Feb 17 '13 at 22:41
  • @haffax I have added a comment to the matrix class example. – juanchopanza Feb 17 '13 at 22:44
  • @Roddy It always depends, yes. ;) Given that the OP's request omits any details on how this data is used, we can't draw conclusions from this. Nevertheless: When working on something this fundamental, design is of utmost importance. Vector of vectors is just too lazy. What if someone uses push_back on a a col vector? This makes no sense. Vector has many member functions that make no sense in this context. 4x4 matrices are common and abundant. You don't want to waste 40% of memory to structure, if you can have it at a lower constant amount. Project don't have just one matrix, they have many. – haffax Feb 17 '13 at 22:56
  • @haffax - cache friendliness is a good point though. array of arrays likely to be terminally bad... – Roddy Feb 17 '13 at 22:57
  • @haffax I take all your points, but concerning the fact that a vector of vectors is not suitable to represent a matrix in the sense that lengths of individual rows can be changed, if we consider that OP starts from an `int**`, I think vector of vectors is an improvement over that, because `int**` could be pretty much anything. – juanchopanza Feb 17 '13 at 23:07
  • @juanchopanza it's true that the code in the OP request is lacking and the request itself is phrased sub-optimally. This still doesn't make option 3 any good for the reasons I tried to outline. IMHO it is bad advice regardless of the quality of the original question. Option 4 is much better IMHO. – haffax Feb 17 '13 at 23:18
  • @haffax allright, I have purged the vector of vectors from my answer. Initially I was concerned that throwing a matrix class at them might be confusing, on the other hand quite often the bad examples are taken on board and used for real code for years and years and we don't want that :-) – juanchopanza Feb 17 '13 at 23:21
2

Method n.2 is allocating a unique block to contain the sequence of all rows. Hence the first row is a pointer to the whole block. If rows==0 you have not space to hold the pointer to the (empty) space, so you cannot make the allocation.

I would steer toward method 4 suggested in the other answer:

class Matrix {
   Matrix(int rows, int cols): rows_(rows), cols_(cols) {
      data_ = new int[rows*cols];
   }

   ~Matrix() {
       delete[] data_;
   }

   int &operator()(int i,int j) {return data_[cols_*i+j];}

   int operator()(int i,int j) const {return data_[cols_*i+j];}

 private:
   int rows_,cols_;
   int *data_;
};
Emanuele Paolini
  • 8,978
  • 3
  • 32
  • 57