The user enters a two-dimensional matrix, and the expected printout is a transposed matrix. For me only works when I enter a square matrix let's say 2 by 2, 3 by 3... For the "normal" matrix it doesn't work. Any help?
Here is my code:
#include<stdio.h>
#include<stdlib.h>
void Trans(int **, int, int);
int main()
{
int n, m;
int **p;
printf("Number of rows: \n");
scanf("%d", &n);
printf("Nuber of columns: \n");
scanf("%d", &m);
p = malloc(n * sizeof(int *));
for (int i = 0; i < n; i++)
{
p[i] = malloc(m * sizeof(int));
}
printf("Elements of matrix: \n");
for (int i = 0; i < n; i++)
{
for (int j = 0; j < m; j++)
{
printf("P[%d][%d] = ", i, j);
scanf("%d", (*(p+i)+j));
}
}
Trans(p, n, m);
return 0;
}
void Trans(int **p, int n, int m)
{
int **a;
a = malloc(m * sizeof(int *));
for (int i = 0; i < m; i++)
{
a[i] = malloc(n * sizeof(int));
}
for (int i = 0; i < m; i++)
{
for (int j = 0; j < n; j++)
{
if (i == j)
a[i][j] = *(*(p+i)+j);
else
a[j][i] = *(*(p+i)+j);
}
}
for (int i = 0; i < m; i++)
{
for (int j = 0; j < n; j++)
{
printf ("%d ", *(*(a+i)+j));
}
puts("");
}
return 0;
}
The biggest problem I see with this code is just that it's so cryptic; your variables are not descriptive, and using pointer math instead of array math on an array just makes it harder to read.
There are definitely places that one-letter variables are okay, even superior, for understanding what's going on, but if I'm having to debug someone else's code I would much rather they err on the side of verbosity and specificity.
To make it clear, I:
For convenience I used literal values in the code for convenience in testing instead of user input.
These are the main changes that helped find this issue:
in main()
allocating the source array with more clearly named extent variables
int num_rows, num_cols;
int** elems;
elems = (int**) malloc(num_rows * sizeof(int*));
for (int i = 0; i < num_rows; i++)
elems[i] = (int*) malloc(num_cols * sizeof(int));
transpose is called
transpose_matrix(elems, num_rows, num_cols);
in transpose_matrix:
the transpose function with more clearly named extent variables
void transpose_matrix(int* elems[], int orig_num_rows, int orig_num_cols)
{
// use variables that explicitly reference the new matrix
int num_rows = orig_num_cols;
int num_cols = orig_num_rows;
** allocating the transposed array with the destination extents **
int** transposed_elems;
transposed_elems = (int**) malloc(num_rows * sizeof(int*));
for (int new_col_index = 0; new_col_index < num_rows; new_col_index++)
transposed_elems[new_col_index] = (int *) malloc(num_cols * sizeof(int));
copying the source matrix to the transposed matrix
for (int row_index = 0; row_index < num_rows; row_index++)
for (int col_index = 0; col_index < num_cols; col_index++)
if (row_index == col_index)
transposed_elems[row_index][col_index] = elems[row_index][col_index];
else
transposed_elems[col_index][row_index] = elems[row_index][col_index];
}
When I run the cleaned-up code, I get an exception:
"Exception thrown at 0x00007FF6AE8B193B in ConsoleApplication1.exe:
0xC0000005: Access violation writing location 0x000001E1FDFDFDFD."
The access violation tells you that you're writing into memory that you shouldn't be writing into. So, first thought should be that your indexes are incorrect. With the more descriptive code, it's easy to figure out what's going on.
transposed_elems has a size of [num_rows][num_cols] which we defined at the top as being the source matrix's old orig_num_cols and the old orig_num_rows.
elems has a size of [orig_num_cols][orig_num_rows] which is equivalent to [num_cols][num_rows].
The exception happens at this line:
// this is trying to copy the matrix element when col_index != row_index
transposed_elems[col_index][row_index] = elems[row_index][col_index];
So, let's look again at that line: you're assigning to transposed_elems[col_index][row_index]. Since this is a row-first array, you're using a column index for a row index, and a row index for a column index. The elements you are trying to copy are elems[row_index][col_index]. This should be reversed since you're accessing the non-transposed array, with column first, and row second. So, that's why if you have anything other than a square array, you will be writing to memory you haven't allocated, and which you don't intend.
There is also another error which won't cause an exception, so it will be harder to find if you're relying on that to debug. When the two indices are equal, you are assigning to [row_index][col_index] and copying from [row_index][col_index].
// this is trying to copy the matrix element when col_index == row_index
transposed_elems[row_index][col_index] = elems[row_index][col_index]`
Because you are only doing it when they are equal to each other (did you have a reason you were trying to check that?) it doesn't write to unallocated memory, but it does assign things incorrectly. Just looking at the meaning of the variables, you can see the transposed matrix is indexed correctly, but the original matrix has the indices reversed.
To make a transposed copy, you only need one statement in your loop to copy them: always assign to [row_index][col_index] and always copy from [col_index][row_index].
for (int row_index = 0; row_index < num_rows; row_index++)
for (int col_index = 0; col_index < num_cols; col_index++)
transposed_elems[row_index][col_index] = elems[col_index][row_index];
Once I make that change, everything works. Here's the output
Elements of matrix: P[0][0] = P[0][1] = P[0][2] = P[0][3] = P[0][4] = P[1][0] = P[1][1] = P[1][2] = P[1][3] = P[1][4] = P[2][0] = P[2][1] = P[2][2] = P[2][3] = P[2][4] = P[3][0] = P[3][1] = P[3][2] = P[3][3] = P[3][4] = P[4][0] = P[4][1] = P[4][2] = P[4][3] = P[4][4] = P[5][0] = P[5][1] = P[5][2] = P[5][3] = P[5][4] = P[6][0] = P[6][1] = P[6][2] = P[6][3] = P[6][4] = P[7][0] = P[7][1] = P[7][2] = P[7][3] = P[7][4] = P[8][0] = P[8][1] = P[8][2] = P[8][3] = P[8][4] = P[9][0] = P[9][1] = P[9][2] = P[9][3] = P[9][4] = 0 1 2 3 4 5 6 7 8 9 1 2 3 4 5 6 7 8 9 10 2 3 4 5 6 7 8 9 10 11 3 4 5 6 7 8 9 10 11 12 4 5 6 7 8 9 10 11 12 13
So, to answer using the original code:
You need to change the loop where you do the assignment to assign to the proper indices, and remove the check for equals which doesn't need to be done.
// remember: [m][n] is the size of the new matrix, and `[n][m]` of the original
// so `[j][i]` is used to index the new one, and `[i][j]` the old one
for (int i = 0; i < m; i++)
for (int j = 0; j < n; j++)
a[j][i] = p[i][j];
A part of developing software is documenting as much of the meaning as you can in your code without making it unbearably verbose. You're not limited to short variable names. If you must, you should be documenting with comments, but that's not as useful, and still can be hard to follow.
It was very easy to find the error just by making the code actually say something. The shortcuts just made it hard to follow. Making it descriptive meant that reading it and seeing whether it made sense became simple.
That's your answer.
For completeness, here's the full version of what I did when I cleaned up the code:
#include<stdio.h>
#include<stdlib.h>
void transpose_matrix(int* elements[], int orig_rows, int orig_cols);
int main()
{
int num_rows, num_cols;
int** elems;
//printf("Number of rows: \n");
//scanf("%d", &num_rows);
//printf("Nuber of columns: \n");
//scanf("%d", &num_cols);
num_rows = 10;
num_cols = 5;
elems = (int**) malloc(num_rows * sizeof(int*));
for (int i = 0; i < num_rows; i++)
{
elems[i] = (int*) malloc(num_cols * sizeof(int));
}
printf("Elements of matrix: \n");
for (int row_index = 0; row_index < num_rows; row_index++)
{
for (int col_index = 0; col_index < num_cols; col_index++)
{
printf("P[%d][%d] = ", row_index, col_index);
//scanf("%d", elems[row_index][col_index]);
elems[row_index][col_index] = row_index + col_index;
}
}
transpose_matrix(elems, num_rows, num_cols);
return 0;
}
void transpose_matrix(int* elems[], int orig_num_rows, int orig_num_cols)
{
int num_rows = orig_num_cols;
int num_cols = orig_num_rows;
int** transposed_elems;
transposed_elems = (int**) malloc(num_rows * sizeof(int*));
for (int new_col_index = 0; new_col_index < num_rows; new_col_index++)
transposed_elems[new_col_index] = (int *) malloc(num_cols * sizeof(int));
for (int row_index = 0; row_index < num_rows; row_index++)
{
for (int col_index = 0; col_index < num_cols; col_index++)
{
if (row_index == col_index)
transposed_elems[row_index][col_index] = elems[row_index][col_index];
else
transposed_elems[col_index][row_index] = elems[row_index][col_index];
}
}
for (int row_index = 0; row_index < num_rows; row_index++)
{
for (int col_index = 0; col_index < num_cols; col_index++)
{
printf("%d ", transposed_elems[row_index][col_index]);
}
puts("");
}
}