patterncMinor
Gauss elimination
Viewed 0 times
eliminationgaussstackoverflow
Problem
The
I was wondering what I can do to make the code shorter (keeping the good style), with better performance and with better memory consumption.
I used linear matrix because I simply don't know how to declare multidimensional array with
I used
```
#include
#include
#define matrix(i,j) matrix[i*(x+1)+j]
int gauss(double matrix, int x, double sol);
int main()
{
int x;
scanf("%i", &x);
double mat = malloc((x + 1) x sizeof(double)), sol = (malloc(x * sizeof(double)));
for (int i = 0; i = 0; k--)
matrix(j, k) -= matrix(i, k) * matrix(j, i) / matrix(i, i);
if (matrix(x, -2) == 0) {
if (matrix(x, -1) == 0)
return -1;
return 1;
}
sol[x - 1] = matrix((x - 1), x) / matrix((x - 1), (x - 1));
for (int i = x - 2; i >= 0; i--) {
double counter = 0;
fo
gauss function takes 3 parameters:- a pointer to the matrix of coefficients
- the number of variables
- a pointer to matrix of solution
#include
#include
int gauss(double* matrix , int x, double* sol);
int main(){
int x;
scanf("%i",&x);
double *mat = malloc((x+1)*x*sizeof(double)), *sol =(malloc(x*sizeof(double)));
for(int i=0;i=0;k--){
matrix[j*(x+1)+k] -= matrix[i*(x+1)+k]*matrix[j*(x+1)+i]/matrix[i*(x+2)];
}
if(matrix[x*(x+1)-2 ] ==0){
if(matrix[x*(x+1)-1]==0) return -1;
return 1;
}
sol[x-1]= matrix[(x-1)*(x+1)+x]/matrix[(x-1)*(x+1)+x-1];
for(int i =x-2; i>=0; i--){
double counter=0;
for(int j=x-1;j>i;j--) counter+= matrix[i*(x+1)+j] *sol[j];
sol[i] = (matrix[i*(x+1)+x]-counter) / matrix[i*(x+2)];
}
return 0;
}I was wondering what I can do to make the code shorter (keeping the good style), with better performance and with better memory consumption.
I used linear matrix because I simply don't know how to declare multidimensional array with
malloc and I don't know how to pass it as a parameter to function.I used
indent -kr -i8 -l1000 -lc1000 for styling and I used preprocessor for simplifying the matrix math but some pieces make no sense as as matrix(x, -2):```
#include
#include
#define matrix(i,j) matrix[i*(x+1)+j]
int gauss(double matrix, int x, double sol);
int main()
{
int x;
scanf("%i", &x);
double mat = malloc((x + 1) x sizeof(double)), sol = (malloc(x * sizeof(double)));
for (int i = 0; i = 0; k--)
matrix(j, k) -= matrix(i, k) * matrix(j, i) / matrix(i, i);
if (matrix(x, -2) == 0) {
if (matrix(x, -1) == 0)
return -1;
return 1;
}
sol[x - 1] = matrix((x - 1), x) / matrix((x - 1), (x - 1));
for (int i = x - 2; i >= 0; i--) {
double counter = 0;
fo
Solution
I think your style is okay, and the algorithm seems nice and short. Good work!
I find short code (that has not been deliberately obfuscated) is almost always easier to understand than long-winded code. Sometimes when I read code, I will have a quick look at the comments, then strip them out so I can see the code more clearly!
Here's some suggestions to improve it. I am not considering the algorithm.
You seem to be using C99 features such as "declare anywhere" (which is arguably not such a great idea)... but anyway, here is how to do dynamic multi-dimensional arrays in C99 with malloc: https://stackoverflow.com/a/12805980/218294 This is the biggest improvement you can make to your code.
You can get rid of
If C99 did not include these features for dynamic multi-dimensional arrays, it might be a good idea to write macros to clarify the array access, e.g.
I suggest adding some spaces at the top level of expressions, this would be easier to read, e.g.:
The second
Since your style includes long lines, you might free both matrices on the same line:
You can exclude the braces in the triple for loop, as there is only one statement inside the loop. You are doing this elsewhere, so do it consistently.
Try to follow K&R (Kernighan and Ritchie) code style. I used this shell command:
here's what this program (GNU Indent) did to your code (no copyright infringement intended!) The code is more readable. I think it put too much space inside the array indexing
If you revise your code, please post it again when you have finished.
I find short code (that has not been deliberately obfuscated) is almost always easier to understand than long-winded code. Sometimes when I read code, I will have a quick look at the comments, then strip them out so I can see the code more clearly!
Here's some suggestions to improve it. I am not considering the algorithm.
You seem to be using C99 features such as "declare anywhere" (which is arguably not such a great idea)... but anyway, here is how to do dynamic multi-dimensional arrays in C99 with malloc: https://stackoverflow.com/a/12805980/218294 This is the biggest improvement you can make to your code.
You can get rid of
malloc and put your arrays on the stack, like this:#include
void print_matrix(int rows, int cols, double M[rows][cols])
{
for (int i=0; i<rows; ++i) {
for (int j=0; j<cols; ++j)
printf("%lf ", M[i][j]);
printf("\n");
}
}
int main(void)
{
int rows = 5;
int cols = rows+1;
double M[rows][cols];
for (int i=0; i<rows; ++i)
for (int j=0; j<cols; ++j)
M[i][j] = i+j;
print_matrix(rows, cols, M);
return 0;
}If C99 did not include these features for dynamic multi-dimensional arrays, it might be a good idea to write macros to clarify the array access, e.g.
#define Matrix(i,j) matrix[i(x+1)+j] then instead of matrix[j(x+1)+k] you could write Matrix(j,k).I suggest adding some spaces at the top level of expressions, this would be easier to read, e.g.:
matrix[i*(x+1)+k] * matrix[j*(x+1)+i] / matrix[i*(x+2)]sol =(malloc(x*sizeof(double))) - no need for outer parens here. You have inconsistent spacing around = here (and similar elsewhere), just a minor thing.The second
%s in printf("The system has %s%s", ... is unnecessary, as the argument is a string literal - just put that string into the format; also put a space before it.Since your style includes long lines, you might free both matrices on the same line:
free(mat); free(sol);You can exclude the braces in the triple for loop, as there is only one statement inside the loop. You are doing this elsewhere, so do it consistently.
Try to follow K&R (Kernighan and Ritchie) code style. I used this shell command:
indent -kr -i8 -l1000 -lc1000here's what this program (GNU Indent) did to your code (no copyright infringement intended!) The code is more readable. I think it put too much space inside the array indexing
[brackets], you could reduce that again (but better use the multi-dimensional arrays).#include
#include
int gauss(double *matrix, int x, double *sol);
int main()
{
int x;
scanf("%i", &x);
double *mat = malloc((x + 1) * x * sizeof(double)), *sol = (malloc(x * sizeof(double)));
for (int i = 0; i = 0; k--) {
matrix[j * (x + 1) + k] -= matrix[i * (x + 1) + k] * matrix[j * (x + 1) + i] / matrix[i * (x + 2)];
}
if (matrix[x * (x + 1) - 2] == 0) {
if (matrix[x * (x + 1) - 1] == 0)
return -1;
return 1;
}
sol[x - 1] = matrix[(x - 1) * (x + 1) + x] / matrix[(x - 1) * (x + 1) + x - 1];
for (int i = x - 2; i >= 0; i--) {
double counter = 0;
for (int j = x - 1; j > i; j--)
counter += matrix[i * (x + 1) + j] * sol[j];
sol[i] = (matrix[i * (x + 1) + x] - counter) / matrix[i * (x + 2)];
}
return 0;
}If you revise your code, please post it again when you have finished.
Code Snippets
#include <stdio.h>
void print_matrix(int rows, int cols, double M[rows][cols])
{
for (int i=0; i<rows; ++i) {
for (int j=0; j<cols; ++j)
printf("%lf ", M[i][j]);
printf("\n");
}
}
int main(void)
{
int rows = 5;
int cols = rows+1;
double M[rows][cols];
for (int i=0; i<rows; ++i)
for (int j=0; j<cols; ++j)
M[i][j] = i+j;
print_matrix(rows, cols, M);
return 0;
}matrix[i*(x+1)+k] * matrix[j*(x+1)+i] / matrix[i*(x+2)]indent -kr -i8 -l1000 -lc1000#include <stdio.h>
#include <stdlib.h>
int gauss(double *matrix, int x, double *sol);
int main()
{
int x;
scanf("%i", &x);
double *mat = malloc((x + 1) * x * sizeof(double)), *sol = (malloc(x * sizeof(double)));
for (int i = 0; i < (x + 1) * x; i++)
scanf("%lf", &mat[i]);
int state = gauss(mat, x, sol);
if (state == 0)
for (int i = 0; i < x; i++)
printf("X%i = %lf\n", i, sol[i]);
else
printf("The system has %s%s", state == -1 ? "infinte number of" : "no", "solution(s)");
free(mat);
free(sol);
return 0;
}
int gauss(double *matrix, int x, double *sol)
{
for (int i = 0; i < x - 1; i++)
for (int j = i + 1; j < x; j++)
for (int k = x; k >= 0; k--) {
matrix[j * (x + 1) + k] -= matrix[i * (x + 1) + k] * matrix[j * (x + 1) + i] / matrix[i * (x + 2)];
}
if (matrix[x * (x + 1) - 2] == 0) {
if (matrix[x * (x + 1) - 1] == 0)
return -1;
return 1;
}
sol[x - 1] = matrix[(x - 1) * (x + 1) + x] / matrix[(x - 1) * (x + 1) + x - 1];
for (int i = x - 2; i >= 0; i--) {
double counter = 0;
for (int j = x - 1; j > i; j--)
counter += matrix[i * (x + 1) + j] * sol[j];
sol[i] = (matrix[i * (x + 1) + x] - counter) / matrix[i * (x + 2)];
}
return 0;
}Context
StackExchange Code Review Q#79453, answer score: 3
Revisions (0)
No revisions yet.