wonder if the following code is correct!.


Hi everyone, I am sorry that the following code is a combination of both C and C++. Regardless of this, If you have any advice to improve or to make the code more modular, please feel free to comment.

//AllocateMatrix.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
#ifndef ALLOCATEMATRIX_H
#define ALLOCATEMATRIX_H
#include <stdlib.h>

template <class dataType>
int allocateData2DbyMalloc(dataType**& data2D, unsigned int nRow, unsigned int nColumn) {
	data2D = (dataType * *)malloc(nRow * sizeof(dataType*)); // calloc(nRow, sizeof(type1*));
	if (data2D == NULL) {
		return EXIT_FAILURE;
	}
	else{
		// Now point each row to an array of nColumn type1 objects
		for (unsigned int i = 0; i < nRow; ++i) {
			data2D[i] = (dataType*)malloc(nColumn * sizeof(dataType)); //calloc(nColumn, sizeof(type1));
			if (data2D[i] == NULL) {
				return EXIT_FAILURE;
			}
		}
	}
	return EXIT_SUCCESS;
}

template <class dataType>
void deAllocateData2DbyFree(dataType** data2D, unsigned int numberOfRow) {
	for (unsigned int row = 0; row < numberOfRow; row++)
		free(data2D[row]);
	free(data2D);
}

#endif 


// Matrix.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
#ifndef MATRIX_H
#define MATRIX_H
struct Matrix
{
	const char* type;
	double** element;
	int nRow;
	int nColumn;
};

Matrix* matrix(double** arr, int nrow, int ncol);
Matrix* matrix(double* arr, int nrow, int ncol);
Matrix* matrix(double* arr, int nrow);
int cmp(const char* s, const char* t);
void deleteMatrix(Matrix* var);
#endif 


// Matrix.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
#include "Matrix.h"
#include "AllocateMatrix.h"

Matrix* matrix(double** arr, int nrow, int ncol)
{
	Matrix* newobj = (Matrix*)malloc(sizeof(*newobj));
//	cout << "arr = " << arr << "    *arr= " << *arr << "	**arr= " << **arr << endl;
	newobj->type = "matrix";
	newobj->element = arr;
//	cout << "newobj->element = " << arr << "    *newobj->element= " << *arr << "	**newobj->element= " << **arr;
	newobj->nRow = nrow;
	newobj->nColumn = ncol;
	return newobj;
}

Matrix* matrix(double* arr, int nrow, int ncol)
{
	Matrix* newobj = (Matrix*)malloc(sizeof(*newobj));
	allocateData2DbyMalloc(newobj->element, 1, 1);
	newobj->type = "vec";
	*newobj->element = arr;
	newobj->nRow = nrow;
	newobj->nColumn = ncol;
	return newobj;
}

// As the 2D pointer is copied into 2D pointer. Hence memory allocation is needed.
Matrix* matrix(double* arr, int nrow)
{
	Matrix* newobj = (Matrix*)malloc(sizeof(*newobj));
	allocateData2DbyMalloc(newobj->element, 1, 1);
	newobj->type = "vec";
	*newobj->element = arr;
	newobj->nRow = nrow;
	newobj->nColumn = 1;
	return newobj;
}

int cmp(const char* s, const char* t)
{
	for (; *s == *t; s++, t++)
		if (*s == '\0')
			return 0;
	return *s - *t;
}

void deleteMatrix(Matrix* var)
{
	if (cmp(var->type, "vec") == 0 && (var->nRow == 1 || var->nColumn == 1))
	{
		deAllocateData2DbyFree(var->element, 1); var->element = '\0';
	}
	else if ((cmp(var->type, "vec") == 0 || cmp(var->type, "matrix") == 0) && var->nRow > 1 && var->nColumn > 1)
	{
		if (cmp(var->type, "vec") == 0)
		{
			if (var->element) deAllocateData2DbyFree(var->element, 1); var->element = '\0';
		}
		else if (cmp(var->type, "matrix") == 0)
		{
			if (var->element) deAllocateData2DbyFree(var->element, var->nRow); var->element = '\0';
		}
	}
	free(var);
}


// main.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
#include <iostream>
#include "Matrix.h"
#include "AllocateMatrix.h"
using namespace std;

int main()
{
	double sarr[3] = {2, 5, 7};
	Matrix* obj = matrix(sarr, 3);
	cout << endl << obj->type << "	" << obj->nRow << "	" << obj->nColumn << endl;
	for (int i = 0; i < 3; i++)
		cout << *(*obj->element + i) << "	";
	cout << endl;
	if(obj)
		free(obj);

	double* darr = (double*)malloc(3 * sizeof(*darr));
	darr[0] = 2;
	darr[1] = 5;
	darr[2] = 7;
	obj = matrix(darr, 3);
	cout << endl << obj->type << "	" << obj->nRow << "	" << obj->nColumn << endl;
	for (int i = 0; i < 3; i++)
		cout << *(*obj->element + i) << "	";
	cout << endl;

	deleteMatrix(obj);

	double matrixElement[2][3] = { {0, 1, 2}, {3, 4, 5} };
	obj = matrix(matrixElement[0], 2, 3);
	cout << endl << obj->type << "	" << obj->nRow << "	" << obj->nColumn << endl;
	for (int i = 0; i < 2; i++)
		for (int j = 0; j < 3; j++)
			cout << *(*obj->element + i * 3 + j) << "	";
	cout << endl;

	double** ptr;
	allocateData2DbyMalloc(ptr, 2, 3);
	for (int i = 0; i < 2; i++)
		for (int j = 0; j < 3; j++)
			ptr[i][j] = matrixElement[i][j];

	obj = matrix(ptr, 2, 3);
	cout << endl << obj->type << "	" << obj->nRow << "	" << obj->nColumn << endl;
	for (int i = 0; i < 2; i++)
		for (int j = 0; j < 3; j++)
			cout << *(*(obj->element + i) + j) << "	";
	cout << endl;
	deleteMatrix(obj);
	return 0;
}
Last edited on
several things stand out...
why the template? Most of your functions talk about doubles.
what is cmp supposed to be doing?
matrix specifically but most 2d in general fare a lot better if you just do 1-d. It takes a long time (in cpu cycles) to allocate and deallocate a ton of small blocks instead of just one block, and there are many other lifts you can get too (transpose in place, resize/reshape easier, simplify some iteratons, save and load to file easier, more)

What did you want from this code in general? It looks like a mix of playing around to learn stuff and yet it feels like it has a purpose, but the purpose is unclear to me.

I didn't see anything incorrect in terms of compiler or major logic errors. But I did not try to compile it or double check every loop etc.
Last edited on
Dear Jonnin,
Thanks. This small code will transfer a matrix or vector to a solver. The matrix or vector might come from a file or another environment as a pointer. The pointer can come as a 1D or 2D. The solver will receive the pointer and put it as a 1D array rather than 2D in heap. The global purpose solver is not only to deal with matrix-vector or matrix-matrix multiplication but also to solve Ax= b more precisely. Actually one of your previous comments inspired me to begin with a 1D array rather than 2D.
Last edited on
1
2
3
4
5
6
7
8
9
10
Matrix* matrix(double* arr, int nrow, int ncol)
{
	Matrix* newobj = (Matrix*)malloc(sizeof(*newobj));
	allocateData2DbyMalloc(newobj->element, 1, 1);    // allocate space for 1x1 matrix
	newobj->type = "vec";
	*newobj->element = arr;       // Leaks the row you allocated above.
	newobj->nRow = nrow;         // Pretend the matrix has nrow's instead of the 1 it actually has.
	newobj->nColumn = ncol;   // pretend it has ncol columns.
	return newobj;
}

On exit, the matrix has room for 1 row and you've copied the first column from the input parameter arr to the matrix, so the matrix has probably taken ownership of that one row? And not the others?

Line 16: if you're allocating a vector then why even let the user pass a parameter specifying the number of columns?

Line 33 Also leaks memory.

Also, your matrix() functions are very fragile, meaning that there are all sorts of hidden conditions:
Matrix* matrix(double** arr, int nrow, int ncol);
arr must be an array of nrow double ptrs and must be allocated on the heap.
Each item in arr must be an array of ncol doubles allocated on the heap.
The returned Matrix takes ownership of arr and the pointers within it. By "Takes ownership" I mean that when you free the matrix, it will free the memory.

Matrix* matrix(double* arr, int nrow, int ncol);
Leaks memory, takes ownership of arr... I think? Sometimes? Depending on nrow and ncol?

Rather than having different storage policies depending on the function used to create the matrix, you should have one consistent policy. If you want different code for vector vs. a matrix, then create different structures for each.

Also, define your memory management policy and stick to it. If you're passing pointers into or out of a function, then DOCUMENT where they point to and who is responsible for deleting them.
Dear Dhayden, Thanks for your message. I am sorry for the late reply. Always glad to have a conversation with an experienced people. Let's start with the code you mentioned at the beginning of your message.

Matrix* matrix(double* arr, int nrow, int ncol)
{
Matrix* newobj = (Matrix*)malloc(sizeof(*newobj));
allocateData2DbyMalloc(newobj->element, 1, 1); // allocate space for 1x1 matrix
newobj->type = "vec";
*newobj->element = arr; // Leaks the row you allocated above.
newobj->nRow = nrow; // Pretend the matrix has nrow's instead of the 1 it actually has.
newobj->nColumn = ncol; // pretend it has ncol columns.
return newobj;
}

The code was checked for the static 1d array (saar[3]), dynamic 1d array(*daar), static 2D array (matrixElement[2][3]) and dynamic 2D array (ptr**). The daar array was allocated and initialized before passing via matrix() function. The matrix function only create a matrix object. Once you call the deleteMatrix(obj) function, the daar array will be deleted at first and afterward, the object will be deleted. The statement allocateData2DbyMalloc(newobj->element, 1, 1) is resposible to copy the pointer from matrix(double* arr, int nrow, int ncol). The reason which impressed me to use 2D pointer rather than 1D pointer, just to keep everything as a matrix. Vector meanly refers as nRowx1. In case of transposed, it will be 1xnColumn. Sometimes, the data element can enter into in c++ world as a 2D array or 1D array from another programming world (Lets say R). 1D array refers that data are stored in contiguous memory but it says that it has nRow(6) and nColumn(8). Sometimes, the data element can enter into in c++ world as a original 2D array (ptr**) where that data are stored in a separate memory chunk corresponds to the row. I find it a bit hard to distinguished ptr* with nRow and nColumn with ptr** with nRow and nColumn when you passed through a function.
Topic archived. No new replies allowed.