1
\$\begingroup\$

I am working on C++ code which builds matrices (3D and 4D) using dynamic arrays. I am using arrays instead of std::vector; as in the future, I might extend this code to some kind of multithreading or parallel process (e.g. CUDA).

In my code, I often initialize and define the source matrix in main.cpp.

Then I have a function (sometimes it may belong to a class) which inputs the matrix along with other variable and I need to return the modified matrix. Which is subsequently used in main.

This is what I usually work with in my code:

    #include <iostream>
    #include <stdio.h>
    #include <stdlib.h>
    using namespace std;

    float ***matFunc ( ***mat3D, int n);

    int main()
    {
      // define dimensions of the matrix 
      int M , N, S = 3;

     //define and initialize the matrix
      float ***myMat = new float **[M];
         for (int i = 0; i< M; i++ ) {
              myMat[i] = new float *[N];
              for ( int j = 0; j < N; j++ ) {
                 myMat[i][j] = new float [S];
                    for (int k = 0; k < S; k++ ) {
                      myMat[i][j[k] = someValue obtained or calculated 
                          }
                   }
              }

         // here I call my member function to perform addition        
         // This is also used if I have more than one matrices

        float ***newMat = matFunc( myMat, int n ); //returns modified matrix

         //deleting the original myMat

         for (int i = 0; i < M; i++) {
             for (int j = 0; j < N; j++) {
               delete[]myMat[i][j];
        }
            delete[] myMat[i];
      }
   delete[]myMat;

      return 0;
     }

     //function definition for matFunc

      float***matFunc(float ***mat, int n ){

       int p, q, r;
       int M, N, S = 3;

           float ***modifiedMat = new float **[M];
                for ( p = 0, p < M; p++ ) {
                   modifiedMat[p] = new float *[N];
                     for ( q = 0; q < n; q++ ) {
                         modifiedmat[p][q] = new float [S];
                          for (r  =0; r < S; r++ ) {
                          modifiedMat[p][q][r] = mat[p][q][r] * do something
                         }
                        }
                      }

           return modifiedMat;
        }

As you can observe, I am always defining and initializing new matrices with help of dynamic arrays. This becomes more troublesome if matrix dimensions are to be increased. I use dynamic arrays due to their faster operations as compared to vectors. But I think there might be some memory management issues with code. As I have observed it consumes more RAM and sometimes my IDE/compiler will complain lack of index or space for integer type.

Could you please suggest how I can improve on this code?

\$\endgroup\$
4
  • \$\begingroup\$ Why is this marked C++. It is closer to C. \$\endgroup\$ Commented Apr 27, 2017 at 19:11
  • 1
    \$\begingroup\$ @LokiAstari Because it obviously is C++, even though it lacks structure, classes, etc., not C lacking structure. \$\endgroup\$ Commented May 6, 2017 at 11:39
  • \$\begingroup\$ @Deduplicator No that is not obvious. It actually looks like C. Replace new/delete with malloc/free its a C program. You would get better advice marking it as C as there are more people watching the C tag giving advice. \$\endgroup\$ Commented May 6, 2017 at 14:41
  • \$\begingroup\$ @LokiAstari: Different advice for sure. Better advice? Only if you want good C instead. \$\endgroup\$ Commented May 6, 2017 at 18:29

3 Answers 3

6
\$\begingroup\$

At first, you should give your program some structure.

Start by indenting all the lines consistently. Ideally, let a program do this automatically. Search for indenter or beautifier.

Then, define a class called Matrix, give it a proper constructor and destructor. This could already help with the memory leak.

Or if you don't like classes, at least define an allocation and a deallocation function for a matrix. But then, it's almost a class anyway.

Finally, fix the syntax errors in your code so that it compiles at least.

Make the code in main so simple to read that you can explain it to a layman. That is, no explicit memory allocation, no details, just the high level view. All the rest of the code needs to go to separate functions.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ clang-format is an excellent formatter. \$\endgroup\$
    – user136614
    Commented Apr 27, 2017 at 3:44
5
\$\begingroup\$

There are two harmless-looking, but severe problems, which in combination will lead to the memory management issues you mentioned.

  1. The line int M, N, S = 3; doesn't work as one might presume. Both M and N are not initialized in your code.

  2. Your program will try to new[] with random values as the number of elements. These values might be very large (or greater than the amount of memory available) or - as the dimension is signed - even negative, with unpredictable consequences.

Most compilers will assist you in finding problems like these two (uninitialized variables, casting signed integer values to size_t), if you enable compiler warnings (e.g. -Wall with g++/clang++).

\$\endgroup\$
1
  • 4
    \$\begingroup\$ Even better with -Wextra -Werror -pedantic \$\endgroup\$ Commented May 18, 2017 at 15:47
2
\$\begingroup\$

Could you provide some reasoning/data about your claim "dynamic arrays due to their faster operations as compared to vectors"?

Given that a std::vector is at its core a dynamic array, I find that kind of general statement highly questionable. What is true is that they have some overhead due to the additional members inside a vector - but that has absolutely nothing to do with operations on vectors.

On the other hand, you gain move semantics and all the fancy stuff that modern C++ provides.

That said, you would definitely benefit most from advanced frameworks like armadillo, eigen or gnu libraries, as they utilize highly optimized code that you will have an extremely hard time (if you can at all) to reach.

If you really want to code it yourself, a matrix class has different possible implementations. (Only for 2D as I am right now too lazy to calculate the indexes)

class myMatrix1 : public std::vector<float> {
    myMatrix1(const size_t x, const size_t y)
        : data(x*y, 0), dimensions({x, y}){}

    float at(const size_t x, const size_t y) const noexcept {
        return data[x * dimension[1] + y];
    }
private:
    std::vector<float>   data;
    std::array<size_t, 2>  dimensions;
}

Basically you map all the data into a long array and then handle the indexes apropriately. This has the advantage of congruent memory. On the other side operations are much harder to imlement.

The other way is your version

class myMatrix2 {
    myMatrix2(const size_t x, const size_t y)
        : data(x, std::vector<float>(y,0))
    }

private:
    std::vector<std::vector<float> >   data;
}

This way has the major advantage, that matrix operations are easier to implement. However, the memory of matrix is not as local as in the other version, which might have performance implications.

\$\endgroup\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.