# Creating an 2D array using pointer/malloc, and then print it out [closed]

I am trying to write 2 functions, one to read the matrix (2D array) and other one to print it out. So far I have:

``````/* Read a matrix: allocate space, read elements, return pointer. The
number of rows and columns are given by the two arguments. */

double **mat = (double **) malloc(sizeof(double *)*rows);
int i=0;
for(i=0; i<rows; i++){
/* Allocate array, store pointer  */
mat[i] = (double *) malloc(sizeof(double)*cols);
//what to do after??

return mat;
}
``````

then the print matrix function, not sure if it is correct

`````` void print_matrix(int rows, int cols, double **mat){
for(i=0; i<rows; i++){    /* Iterate of each row */
for(j=0; j<cols; j++){  /* In each row, go over each col element  */
printf("%f ",mat[i][j]); /* Print each row element */
}
}}
``````

and here is the main function I am using to run:

``````#include <stdio.h>
#include <stdlib.h>

void print_matrix(int rows, int cols, double **mat);
void free_matrix(int rows, double **mat);

int main(){

double **matrix;
int rows, cols;
/* First matrix */
printf("Matrix 1\n");
printf("Enter # of rows and cols: ");
scanf("%d %d",&rows,&cols);
printf("Matrix, enter %d reals: ",rows*cols);
printf("Your Matrix\n");  /* Print the entered data */
print_matrix(rows,cols,matrix);
free_matrix(rows, matrix);   /* Free the matrix */

return 0;}
``````
-
Please note that as per the faq, any code should be working, and actual code from a project rather than pseudo-code or example code. –  Glenn Rogers Oct 23 '12 at 8:21
Note that `scanf` returns the number of values it read. In `scanf("%d %d",&rows,&cols);` it should return 2. If it returns 0 or 1 then the user entered non-numeric data. Subsequent use of `scanf` to read numeric data will fail until the non-numeric data is read or flushed (see `fpurge()`). `scanf` can also return -1 if the input stream is closed. –  William Morris Oct 23 '12 at 18:09

## closed as off topic by Glenn Rogers, palacsint, mseancole, Jeff Vanzella, Trevor PilleyOct 23 '12 at 17:53

Questions on Code Review Stack Exchange are expected to relate to code review request within the scope defined by the community. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about reopening questions here.If this question can be reworded to fit the rules in the help center, please edit the question.

It seems OK to me as far as it goes. A couple of suggestions though:

1. `read_matrix` may be better split up into two functions, one to create it and the other to read the contents from the source, even if the `create_matrix` function is called from `read_matrix` - and you need to be aware of the possibility that the allocation may fail.
2. On the other hand, you may wish to consider just allocating a single block of size rows*cols of memory, and indexing it via `y*rows + x` rather than allocating several smaller blocks.
3. For 'neatness', you should consider using `calloc( number_of_elements, sizeof (double) )` instead of `malloc`, just to give a (possibly) sensible initial value.
4. `print_matrix` could do with some layout changes, such that each row is on a separate line, and the corresponding cells line up under each other, something like `%10.3f` (but would depend on the type of data expected).
-

Its fine for what it does.

But you should try and group all the relavant information into a single structure so that you pass it round as one item (this will prevent typing mistakes).

``````typedef struct TwoDArray
{
int      cols;
int      rows
doubel** data;
} TwoDArray;

``````

Declare one variable per line:

``````int rows, cols;   /* Any industry style guide will force you to split
this over two lines
*/
``````

You are not being payed to conserve vertical space. You are being payed to make the code as readable and maintainable as possible.

You need to pick a brace style and be consistent. There are a couple of styles out there (I can't argue that any one is better than another). So just find a style you like but then by consistent when using it:

``````Stuff       /* Lined up Style */
{
Plop
}

Stuff  {    /* K&R up Style */
Plop
}

Stuff       /* indented Style (rarer than the other two) */
{
Plop
}
``````

The only thing I would complain about (and its not major) is neatness. Use more white space to break the code into sections. This makes it easier for the reader to group stuff together into logical blocks.

``````int main()
{
double **matrix;
int      row;
int      cols;

printf("Matrix 1\n");
printf("Enter # of rows and cols: ");
scanf("%d %d",&rows,&cols);

printf("Matrix, enter %d reals: ",rows*cols);

print_matrix(rows,cols,matrix);

free_matrix(rows, matrix);

return 0;
}
``````

``````free_matrix(rows, matrix);   /* Free the matrix */
``````

One could even argue worse then useless as the comments over time may stray from the code (or it takes extra work to keep the comments lined up with the work).

The code should explain itself (as it does: the function free_matrix() does not really need any explanation). You should use comments to explain why (or potentially how).

``````/*
*    Parameters:
*       rows/cols    the size of the matrix to be allocated.
*    Returns:
*        A pointer to an a data structure such that
*        the expression M[a][b] to be valid.
*        Where (0 <= a < rows)  && (0 <= b < cols)
*
*    This structure should be released using the function free_matrix()
*/