# Gauss-Seidal implementation

The code is an implementation of the Gauss–Seidel method. I would like a general review of the code.

PS: I use code::blocks IDE

//generalized code for gauss seidal iteration method
#include<stdio.h>
#include<malloc.h>

#define chk_end 0.01

int row_lim=0,col_lim=0;

float** partial_pivot(float** mat,int row,int col);//finds the pivot of matrix
int condition(float* result,float* temp,int row);//checks the terminating condition
float* final_result(float** mat,float* result,int row,int col);//calculates final result
float** create_mat(float** mat,int row,int col);//creates a matrix of row X col
float** feed_mat(float** mat,int row,int col);//inputs value in matrix
void disp_mat(float** mat,int row,int col);//displays matrix

float** partial_pivot(float** mat,int row,int col)
{
float pivot;
int i=0,j=0,k=0,l=0;

if(row_lim<row)
{
for(i=0,j=col_lim;i<row-1;i++)
{
for(l=row_lim;l<row-i-1;l++)
{
if(mat[l][j]<mat[l+1][j])
{
for(k=0;k<col;k++)
{
pivot=mat[l+1][k];
mat[l+1][k]=mat[l][k];
mat[l][k]=pivot;
}
}
}
}
row_lim++;
col_lim++;
partial_pivot(mat,row,col);
}

return mat;
}

int condition(float* result,float* temp,int row)
{
int i=0,flag = 0;

for(i=0;i<row;i++)
{
if(result[i] > temp[i])
{
if((result[i]-temp[i]) < chk_end)
{
flag = 1;
}
else
{
flag = 0;
break;
}
}

else
{
if((temp[i]-result[i]) < chk_end)
{
flag = 1;
}
else
{
flag = 0;
break;
}
}
}

return flag;
}

float* final_result(float** mat,float* result,int row,int col)
{
int k=0,i=0,j=0;
float T = 0;
float *temp;

temp=(float*)malloc((row)*sizeof(float));//Edit: row-1 changed to row
for(i=0;i<row;i++)
{
temp[i] = 0;
}

printf("Processing result ... \n");

do
{
for(k=0;k<row;k++)
temp[i] = result[i];    //was a logical error, sorry
{
for(i=0;i<row;i++)
{
for(j=0;j<(col-1);j++)
{
if(i==j)
{

}
else
{
T += (mat[i][j])*(temp[j]);
}
}
result[i] = ((mat[i][col-1] - T)/(mat[i][i]));
//temp[i] = result[i];
T=0;
}
}
}
while(condition(result,temp,row) == 0);

free(temp);

return result;
}

float** create_mat(float** mat,int row,int col)
{
int i=0;

mat=(float**)malloc((row)*sizeof(float*));//Edit: row-1 changed to row
if(mat)
{
for(i=0;i<row;i++)
{
mat[i]=(float*)malloc((col)*sizeof(float));//Edit: col-1 changed to col
}
}

return mat;
}

float** feed_mat(float** mat,int row,int col)
{
int i=0,j=0;

for(i=0;i<row;i++)
{
for(j=0;j<col;j++)
{
printf("Enter mat[%d][%d] element:\n",i,j);
scanf("%f",&mat[i][j]);
}
}

return mat;
}

void disp_mat(float** mat,int row,int col)
{
int i=0,j=0;

for(i=0;i<row;i++)
{
for(j=0;j<col;j++)
{
printf("\t%f",mat[i][j]);
}
printf("\n");
}

}

int main()
{
float **mat,*result;
int i=0,row=0,col=0;

mat=NULL;

printf("Enter the number of Equations :\n");
scanf("%d",&row);
col=row+1;

result = (float*)malloc((row)*sizeof(float));//Edit: row-1 changed to row
for(i=0;i<row;i++)
{
result[i] = 0;
}

mat=create_mat(mat,row,col);

printf("Enter the values in the %d x %d matrix:\n",row,col);
mat=feed_mat(mat,row,col);

printf("matrix entered by you:\n");
disp_mat(mat,row,col);

mat=partial_pivot(mat,row,col);

printf("pivoted matrix entered:\n");
disp_mat(mat,row,col);

result = final_result(mat,result,row,col);

printf("\nResult is : \n");
for(i=0;i<row;i++)
{
printf("  x[%d]   %f",i,result[i]);
}

for(i=0;i<row;i++)
{
free(mat[i]);
}
free(mat);

free(result);

return 0;
}


A word of warning: I am very much a C newbie myself, so it may be good to take my advice with a grain of salt.

## Formatting

The only good thing I can say about your formatting is that it's consistent. But a 1-space indent really isn't readable. I recommend 4 spaces instead. Please put some whitespace around your operators: a space after every comma, a space on each side of =, -, +, *, …. Don't cram multiple assignments on one line: I find

int i, j, k;


or

int i = 0;
int j = 0;
int k = 0;


acceptable, but not int i=0,j=0,k=0;.

If you are targetting a C89-compatible environment, this is as far as we can go. I recommend you move on (e.g. to C++ or C99), where we can put the declaration of for-loop variables inside the loop itself:

for (int i = 0; i < whatever; i++)


… and we also don't have to put all declarations at the top of each scope – just declare your variables at the point of first use.

## malloc and error handling

Always check the return value of malloc:

float *temp = (float*) malloc(row * sizeof(float));
if (temp == NULL)
{
/* return some error code */
}


## C isn't object oriented, but whatever

You always bundle the three variables mat, row, col. I suggest you put those into a struct, and pass this single struct around instead:

typedef struct {
float **mat;
int rows;
int cols;
} Matrix;


I would then create a Matrix_new function that creates and populates an instance, e.g.

// this code assumes C99. Ignore my more compact brace style.

/**
* Allocates a new Matrix instance. Returns a pointer to the instance on
* success, a NULL pointer on failure.
*
* A Matrix must always be freed via Matrix_destroy(instance)!
*/

Matrix*
Matrix_new(int rows, int cols) {

Matrix* matrix = (Matrix*) malloc(sizeof(Matrix));
if (matrix == NULL) {
return NULL;
}

float** mat = (float**) malloc(rows * sizeof(float*));
if (mat == NULL) {
free(matrix);
return NULL;
}

for (int i = 0; i < rows; i++) {
float* row = (float*) malloc(cols * sizeof(float));
if (row == NULL) {
// if a row allocation fails, we have to free the previous rows.
i--;
for (; i >= 0; i--) {
free(mat[i]);
}
free(mat);
free(matrix);
return NULL;
}

}

matrix->mat  = mat;
matrix->rows = rows;
matrix->cols = cols;
return matrix;
}


and of course, a mirroring Matrix_destroy:

/**
* Safely frees a Matrix instance.
*/

int
Matrix_destroy(Matrix* matrix) {

if (matrix == NULL) {
return 0;
}

float** mat = matrix->mat;
int rows = matrix->rows;

for (int i = 0; i < rows; i++) {
free(mat[i]);
}

free(mat);
free(matrix);

return 1;
}


Then, you can rewrite your other methods to take advantage of this bundling.

## Return values

Very often (e.g. in final_result, create_mat, feed_mat), you return a pointer from your function even when that pointer was an argument to that function. It is generally preferable to return an error code, and to return values via pointers/out-arguments.

For example, final_result could be changed to:

/**
* Calculates the final result.
*
* Returns an error code.
*
* After successful execution, the out_result pointer will point to the
* result, which is a float* of self->rows length. The caller owns the
* result, and is responsible for calling free(*out_result).
*
* Example usage:
*
*     float *result;
*     if (! final_result(matrix, &result)) {
*         // abort with error
*     }
*     // do something with result:
*     for (int i = 0; i < matrix->rows; i++) {
*         printf("%f\n", result[i]);
*     }
*     free(result);
*/

int
final_result(Matrix* self, float** out_result) {
if (self == NULL) {
return 0;
}

float* temp = (float*) malloc(self->rows * sizeof(float));
if (temp == NULL) {
return 0;
}

float* result = (float*) malloc(self->rows * sizeof(float));
if (result == NULL) {
free(temp);
return 0;
}

for (int i = 0; i < self->rows; i++) {
temp[i]   = 0;
result[i] = 0;
}

printf("Processing result...\n");

do {
for (int i = 0; i < self->rows; i++) {
float T = 0;
for (int j = 0; j < self->cols - 1; j++) {
if (i != j) {
T += self->mat[i][j] * temp[j];
}
}
result[i] = (self->mat[i][self->cols - 1] - T) / self->mat[i][i];
temp[i] = result[i];
}
} while (condition(result, temp, self->rows) == 0);

free(temp);
*out_result = result;
return 1;
}


## Miscellaneous Helper Functions

### memset

When zeroing all elements in some data structure, you can often use memset from string.h instead:

temp=(float*)malloc((row)*sizeof(float));//Edit: row-1 changed to row
for(i=0;i<row;i++)
{
temp[i] = 0;
}


becomes

float* temp = (float*) malloc(row * sizeof(float));
if (temp == NULL) {
// return error code
}
memset(temp, 0, row * sizeof(float));


### fabsf

If you are interested in an absolute value of a float, use the fabsf function from math.h. With it, your condition function simplifies to:

int
condition(float* result, float* temp, int row) {
int flag = 0;

for (int i = 0; i < row; i++) {
if (fabsf(temp[i] - result[i]) < chk_end) {
flag = 1;
}
else {
flag = 0;
break;
}
}

return flag;
}

• dude heartly thanks a highly detailed way of making my program optimized and thanks for spending time on it,, i am having one doubt on your point please just elaborate what you want to say through it i.e. /*Return values Very often (e.g. in final_result, create_mat, feed_mat), you return a pointer from your function even when that pointer was an argument to that function. It is generally preferable to return an error code, and to return values via pointers/out-arguments. */ //what do you mean by return values via pointers/out-arguments// Commented Mar 25, 2014 at 3:03
• @Amitesh I added an example rewrite of final_result that showcases this: The return value is a status code, whereas the result is returned via a pointer argument. In this simple case, this isn't really necessary, but quite often you'll want to return multiple values from a function: pointer arguments are the best way to do that.
– amon
Commented Mar 25, 2014 at 10:13
• thanks dude got your point and will optimize my code by using your tips thanks. Commented Mar 25, 2014 at 15:09