2
\$\begingroup\$

How can I make the code look cleaner, and what can I do to reduce the number of lines I have written?

This program calculates the addition, subtraction and multiplication of matrices as well as scalar multiplication.

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>


//User Defined Function Declaration
void readMatrix(int array[10][10], int rows, int colums);
void printMatrix(int array[10][10], int rows, int colums);
void matrixAddSub(int arrayone[10][10], int arraytwo[10][10], int rows, int colums, int mul);
void matrixScalarMultiply(int array[10][10], int scalar, int rows, int colums);
void matrixMultiply(int arrayone[10][10], int arraytwo[10][10], int rowsA, int columsA, int columsB);

int main(void){

    int i, j, k; //used in for loops

    int matrixA[10][10]; // initialized at 10 just to have it initialized
    int matrixB[10][10];
    int rowA, colA;
    int rowB, colB;
    int operation;//used in swtich statements
    char again = 'Y';
    int scalar = 0;
    int add = 1;
    int sub = -1;

    while (again == 'Y'){


        //this is the operation menu just type A, B, C or D to calculate
        printf("\nOperation Menu\n");
        printf("\t1. to Add\n");
        printf("\t2. to Subtract\n");
        printf("\t3. to Scalar Multiply\n");
        printf("\t4. to Multiply two matrices\n");
        printf("Enter yout choice: ");
        scanf(" %d", &operation);


        switch (operation){

        case 1:
            printf("\nEnter the #rows and #cols for matrix A: ");
            scanf("%d%d", &rowA, &colA);

            printf("Enter the #rows and #cols for matrix B: ");
            scanf("%d%d", &rowB, &colB);

            while ((rowA != rowB) && (colA != colB)){
                printf("\nMatrices must be the same size\n");
                printf("\nEnter the #rows and #cols for matrix A: ");
                scanf("%d%d", &rowA, &colA);

                printf("Enter the #rows and #cols for matrix B: ");
                scanf("%d%d", &rowB, &colB);

            }


            printf("\n\tEnter elements of Matrix A a %d x %d matrix.\n", rowA, colA); // with the %d we remember the user the dimentions of the array
            readMatrix(matrixA, rowA, colA);
            printf("\n\t\tMatrix A\n\n");
            printMatrix(matrixA, rowA, colA);


            printf("\n\tEnter elements of Matrix B a %d x %d matrix.\n", rowB, colB); // with the %d we remember the user the dimentions of the array
            readMatrix(matrixB, rowB, colB);
            printf("\n\t\tMatrix B\n\n");
            printMatrix(matrixB, rowB, colB);


            printf("\nThe Sum of matrixA + matrixB is : \n");
            matrixAddSub(matrixA, matrixB, rowA, colA, add);

            break;

        case 2:

            printf("\nEnter the #rows and #cols for matrix A: ");
            scanf("%d%d", &rowA, &colA);

            printf("Enter the #rows and #cols for matrix B: ");
            scanf("%d%d", &rowB, &colB);

            while ((rowA != rowB) && (colA != colB)){
                printf("\nMatrices must be the same size\n");
                printf("\nEnter the #rows and #cols for matrix A: ");
                scanf("%d%d", &rowA, &colA);

                printf("Enter the #rows and #cols for matrix B: ");
                scanf("%d%d", &rowB, &colB);
            }

            printf("\n\tEnter elements of Matrix A a %d x %d matrix.\n", rowA, colA); // with the %d we remember the user the dimentions of the array
            readMatrix(matrixA, rowA, colA);
            printf("\n\t\tMatrix A\n\n");
            printMatrix(matrixA, rowA, colA);

            printf("\n\tEnter elements of Matrix B a %d x %d matrix.\n", rowB, colB); // with the %d we remember the user the dimentions of the array
            readMatrix(matrixB, rowB, colB);
            printf("\n\t\tMatrix B\n\n");
            printMatrix(matrixB, rowB, colB);

            printf("\nThe difference between matrixA - matrixB is : \n");
            matrixAddSub(matrixA, matrixB, rowA, colA, sub);
            break;

        case 3:

            printf("\nEnter the scalar: ");
            scanf("%d", &scalar);
            printf("\nThe scalar is: %d ", scalar);


            printf("\nEnter the #rows and #cols for matrix A: ");
            scanf("%d%d", &rowA, &colA);

            printf("\n\tEnter elements of Matrix A a %d x %d matrix.\n", rowA, colA); // with the %d we remember the user the dimentions of the array
            readMatrix(matrixA, rowA, colA);
            printf("\n\t\tMatrix A\n\n");
            printMatrix(matrixA, rowA, colA);

            printf("\nThe scalar multiplication between matrixA * %d is: \n", scalar);
            matrixScalarMultiply(matrixA, scalar, rowA, colA);

            break;

        case 4:
            //when mulotiplying arrays matrixA colum # has to equal matrixB row #
            printf("\nEnter the #rows and #cols for matrix A: ");
            scanf("%d%d", &rowA, &colA);

            printf("Enter the #rows and #cols for matrix B: ");
            scanf("%d%d", &rowB, &colB);

            // Column of first matrix should be equal to column of second matrix and
            while (colA != rowB)
            {
                printf("\n\nError! column of first matrix not equal to row of second.\n\n");
                printf("\nEnter the #rows and #cols for matrix A: ");
                scanf("%d%d", &rowA, &colA);

                printf("Enter the #rows and #cols for matrix B: ");
                scanf("%d%d", &rowB, &colB);
            }

            // Storing elements of first matrix.
            printf("\n\tEnter elements of Matrix A a %d x %d matrix.\n", rowA, colA); // with the %d we remember the user the dimentions of the array
            readMatrix(matrixA, rowA, colA);
            printf("\n\t\tMatrix A\n\n");
            printMatrix(matrixA, rowA, colA);

            // Storing elements of second matrix.
            printf("\n\tEnter elements of Matrix B a %d x %d matrix.\n", rowB, colB); // with the %d we remember the user the dimentions of the array
            readMatrix(matrixB, rowB, colB);
            printf("\n\t\tMatrix A\n\n");
            printMatrix(matrixB, rowB, colB);

            //multiplyng arrays
            matrixMultiply(matrixA, matrixB, rowA, colA, colB);

            break;

        default:
            printf("\nIncorrect option! Please choose a number 1-4.");
            break;
        }

        printf("\n\nDo you want to calculate again? Y/N\n");
        scanf(" %c", &again);
        again = toupper(again);
    }
    printf("\n\nGoodbye!\n\n");

    return 0;
}


//User Defined Function Definition
void readMatrix(int array[10][10], int rows, int colums){
    int i, j;
    for (i = 0; i < rows; i++){
        printf("\t%d entries for row %d: ", colums, i + 1);
        for (j = 0; j < colums; j++){
            scanf("%d", &array[i][j]);
        }
    }

    return;
}

void printMatrix(int array[10][10], int rows, int colums){
    int i, j;

    for (i = 0; i < rows; i++) {
        for (j = 0; j < colums; j++){
            printf("\t%d", array[i][j]);
        }
        printf("\n");
    }
}


void matrixAddSub(int arrayone[10][10], int arraytwo[10][10], int rows, int colums, int mul){
    int i, j;
    int sumM[10][10];
    int scaM[10][10];
    for (i = 0; i < rows; i++){
        for (j = 0; j < colums; j++){
            scaM[i][j] = mul * arraytwo[i][j];
            }
        }


    for (i = 0; i < rows; i++){
        for (j = 0; j < colums; j++){
            sumM[i][j] = arrayone[i][j] + scaM[i][j];
            printf("\t%d", sumM[i][j]);
        }
        printf("\n");
    }
}


void matrixScalarMultiply(int array[10][10], int scalar, int rows, int colums){
    int i, j;
    int scaM[10][10];
    for (i = 0; i < rows; i++){
        for (j = 0; j < colums; j++){
            scaM[i][j] = scalar * array[i][j];
            printf("%d\t", scaM[i][j]);
        }
        printf("\n");
    }

}

void matrixMultiply(int arrayone[10][10], int arraytwo[10][10], int rowsA, int columsA,int columsB){
    int i, j, k;
    int mulM[10][10];

    // Initializing all elements of result matrix to 0
    for (i = 0; i<rowsA; ++i)
        for (j = 0; j<columsB; ++j)
        {
            mulM[i][j] = 0;
        }

    // Multiplying matrices a and b and
    // storing result in result matrix
    for (i = 0; i<rowsA; ++i)
        for (j = 0; j<columsB; ++j)
            for (k = 0; k<columsA; ++k)
            {
                mulM[i][j] += arrayone[i][k] * arraytwo[k][j];
            }
    printf("\nOutput Matrix:\n");
    for (i = 0; i<rowsA; ++i)
        for (j = 0; j<columsB; ++j)
        {
            printf("\t%d ", mulM[i][j]);
            if (j == columsB - 1)
                printf("\n\n");
        }
}
\$\endgroup\$
0
\$\begingroup\$
  1. You can initialize arrays to zero, like this:

    int matrixA[10][10]={};
    int matrixB[10][10]={0}; //or like this
    //but it only works for 0, not any other values.
    
  2. In the main() function, integers i,j,k have no use so they must be removed.

  3. The function matrixAddSub() multiplies the value of integer mul to the second matrix and then adds it to the first matrix. What if the first matrix is initialized to 0? In that case we get the scalar product of mul and the second matrix. So we can use this function instead of matrixScalarMultiply(). Just initialize both matrices to zero after entering the outermost while() loop, and then:

    case 3:
        ... //take input for matrixB
        ... //and input of integer `scalar`
        matrixAddSub(matrixA,matrixB,rowB,colB,scalar);
            // we pass scalar instead of 1 or -1
            // matrixA is initialized with 0, so adding it will not effect the output
    
  4. We do not need to make integers add=1 and sub=-1 its easier to understand if we pass 1 and -1 directly to matrixAddSub() for addition and subtraction. It saves a little memory too.

  5. In the function matrixMultiply() we can initialize mulM without a loop as mentioned in point 1. [Try to keep your variable names easy to be edited later when needed, like mat1 instead of arrayone and you can reuse the same word for different variables if they are in different functions.]

    Also, we can combine the calculation loop and the printing loop into a single loop. After making those changes, it will look like this:

    void matrixMultiply(int mat1[10][10], int mat2[10][10], int rowsA, int colsA, int colsB)
    {
        int i, j, k;
        int mulM[10][10] = {};
    
        printf("\nOutput Matrix:\n");
        for (i = 0; i<rowsA; ++i)
        {
            for (j = 0; j < colsB; ++j)
            {
                for (k = 0; k < colsA; ++k)
                    {
                        mulM[i][j] += mat1[i][k] * mat2[k][j];
                    }
                printf("\t%d ", mulM[i][j]);
            }
            printf("\n\n"); //newline after every row
        }
    }
    
  6. You will notice that after making change #5, we do not need to waste memory for storing values in the temporary matrix mulM, it is very big, so we can replace it with an integer which will reset itself to 0 at start of every column (j). Doing the above modification gives:

    void matrixMultiply(int mat1[10][10], int mat2[10][10], int rowsA, int colsA, int colsB)
    {
        int i, j, k;
        int x;
    
        printf("\nOutput Matrix:\n");
        for (i = 0; i<rowsA; ++i)
        {
            for (j = 0; j < colsB; ++j)
            {
                x=0; //initialising x to 0 before every calculation starts
                for (k = 0; k < colsA; ++k)
                    {
                        x += mat1[i][k] * mat2[k][j];
                    }
                printf("\t%d ", x);
            }
            printf("\n\n");
        }
    }
    
  7. You can ask the user for entering elements of the matrix from within the readMatrix() function, by making it accept a character. This will reduce the need to type the same requesting line many times. This is especially useful if you are going to improve this program later to handle more than two matrices at once and if you still want to name them with letters.

    void readMatrix(int array[10][10], int rows, int colums, char name)
    {
        int i, j;
        printf("\n\tEnter elements of Matrix %c a %d x %d matrix.\n", name, rows, colums);
        ....// loop for taking input of elements
    }
    

    The same thing can be done with printMatrix() function after which you can take input and print any matrix in two lines:

        readMatrix(matrixA,rowsA,colsA,'A');
        printMatrix(matrixA,rowsA,colsA,'A');
    
  8. Use do-while loops instead of while loops if you need the body of your loops to execute atleast once. So while taking input of rows and columns

    do{
        //take input;
    } while( /* condition for wrong input */ );
    
  9. The most important changes are required in your menu system. There are four operations and we need to input 2 arrays for case 1, case 2, case 4, but for case 3 we need to input 1 array and 1 integer. So we can take input for case 1, 2 and 4 together, and or case 3 separately. But that will change the entire menu system, so face of the program will not be recognisable. :P Heres what can be done. I have replaced most code with ellipsis(...) and comments:

    char correct; //Will be used to check for correct inputs
    ... //all initializations
    while(again=='Y')
    {
      //print the menu and take input of 'operation', after that
      if(operation != 3)  //case 1, 2, 4
      {
            correct='N';
            do
            {
                //take input of rows and columns of matrices
                //now we check for correct inputs of rows and columns
                switch(operation){
                case 1:
                case 2: if((rowsA==rowsB) && (colsA==colsB)) correct='Y';
                        else //print error message
                        break;
                default: if(colsA == rowsB) correct='Y';
                         else //print error message
                         break;
                }
            }while(correct == 'N');
    
            //inputting values of both matrices
    
            //after that, time for processing
            switch(operation) {
            case 1:  //function call for addition
            case 2:  //function call for subtraction
            default: //function call for matrix multiplication
            }
        }
        else if (operation==3)
        {
            //do whats needed for case 3
        }
        else
        {
            //warning about wrong input
        }
        ...
    

Much shorter programs are possible with varying complexity

\$\endgroup\$
  • \$\begingroup\$ its x, thx for pointing out sir. I have corrected it \$\endgroup\$ – Subham Burnwal Mar 25 '18 at 1:10
2
\$\begingroup\$

The DRY principle is a good way to make your code more maintainable and clear, by having each operation or data written only once in your code. It could make your code shorter, but more importantly, it will match better how your brain understand the program (changing a function will change it across all of your code, changing a function would not require a change in other logically unrelated element, etc').

First, you could extract getting the matrix:

    printf("\n\tEnter elements of Matrix A a %d x %d matrix.\n", rowA, colA); // with the %d we remember the user the dimentions of the array
    readMatrix(matrixA, rowA, colA);
    printf("\n\t\tMatrix A\n\n");
    printMatrix(matrixA, rowA, colA);

Into a function:

void readMatrixWithInstructions(int matrix[10][10], char name[],int rows,int cols){
    printf("\n\tEnter elements of Matrix %s a %d x %d matrix.\n",name, rows, cols); // with the %d we remember the user the dimentions of the array
    readMatrix(matrix, rows, cols);
    printf("\n\t\tMatrix %s\n\n",name);
    printMatrix(matrix, rows, cols);
}

For example, in case 3 it would look like that:

case 3:
    printf("\nEnter the scalar: ");
    scanf("%d", &scalar);
    printf("\nThe scalar is: %d ", scalar);

    printf("\nEnter the #rows and #cols for matrix A: ");
    scanf("%d%d", &rowA, &colA);
    readMatrixWithInstructions(matrixA,"A",rowA,colA);

    printf("\nThe scalar multiplication between matrixA * %d is: \n", scalar);
    matrixScalarMultiply(matrixA, scalar,rowA,colA);
break;

Second, You could also isolate getting the dimensions of the matrix:

    printf("\nEnter the #rows and #cols for matrix A: ");
    scanf("%d%d", &rowA, &colA);

Into another function:

void readDimentions(struct dimensions* d, char name[]){
    printf("\nEnter the #rows and #cols for matrix %s: ",name);
    scanf("%d%d",&( d->row ), &( d->col ));
}

(I used a struct that I created to bundle row and column to one "variable", because they always come togher):

struct dimensions{
    int row;
    int col;
};

So now you change case 4 to be:

case 4:
    //when multiplying arrays matrixA colum # has to equal matrixB row #
    readDimentions(&da, "A");
    readDimentions(&db, "B");

    // Column of first matrix should be equal to column of second matrix and
    while (da.col != db.row)
    {
        printf("\n\nError! column of first matrix not equal to row of second.\n\n");
        readDimentions(&da, "A");
        readDimentions(&db, "B");
    }
    readMatrixWithInstructions(matrixA,"A",da.row,da.col);
    readMatrixWithInstructions(matrixB,"B",db.row,db.col);

    //multiplyng arrays
    matrixMultiply(matrixA, matrixB, da.row, da.col, db.col);
break;

Last, Both case 1 and 2 starts with trying to get the user to type the same dimensions twice, so you could extract that part into a function:

struct dimensions readTwoIdenticalDimensions(){
    struct dimensions da,db;
    readDimentions(&da, "A");
    readDimentions(&db, "B");

    while ((da.row != db.row) || (da.col != db.col)){ // fixed from && to ||
        printf("\nMatrices must be the same size\n");

        readDimentions(&da, "A");
        readDimentions(&db, "B");
    }
    return da;
}

We can use it in case 1:

case 1:
    d = readTwoIdenticalDimensions();
    readMatrixWithInstructions(matrixA,"A",d.row,d.col);
    readMatrixWithInstructions(matrixB,"B",d.row,d.col);

    printf("\nThe Sum of matrixA + matrixB is : \n");
    matrixAddSub(matrixA, matrixB, d.row,d.col, add);
break;

Using the functions in all the cases:

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>

struct dimensions{
    int row;
    int col;
};

//User Defined Function Declaration
struct dimensions readTwoIdenticalDimensions();
void readDimentions(struct dimensions *dimensions, char name[]);
void readMatrix(int array[10][10], int rows, int colums);
void readMatrixWithInstructions(int matrix[10][10], char name[],int rows,int cols);
void printMatrix(int array[10][10], int rows, int colums);
void matrixAddSub(int arrayone[10][10], int arraytwo[10][10], int rows, int colums, int mul);
void matrixScalarMultiply(int array[10][10], int scalar, int rows, int colums);
void matrixMultiply(int arrayone[10][10], int arraytwo[10][10], int rowsA, int columsA, int columsB);

int main(void){

    /* int i, j, k; //used in for loops */

    int matrixA[10][10]; // initialized at 10 just to have it initialized
    int matrixB[10][10];
    /* int rowA, colA; */
    /* int rowB, colB; */
    int operation;//used in swtich statements
    char again = 'Y';
    int scalar = 0;
    int add = 1;
    int sub = -1;

    while (again == 'Y'){


        //this is the operation menu just type A, B, C or D to calculate
        printf("\nOperation Menu\n");
        printf("\t1. to Add\n");
        printf("\t2. to Subtract\n");
        printf("\t3. to Scalar Multiply\n");
        printf("\t4. to Multiply two matrices\n");
        printf("Enter yout choice: ");
        scanf(" %d", &operation);

        struct dimensions d;
        struct dimensions da,db;

        switch (operation){

        case 1:
            d = readTwoIdenticalDimensions();
            readMatrixWithInstructions(matrixA,"A",d.row,d.col);
            readMatrixWithInstructions(matrixB,"B",d.row,d.col);

            printf("\nThe Sum of matrixA + matrixB is : \n");
            matrixAddSub(matrixA, matrixB, d.row,d.col, add);

            break;

        case 2:

            d = readTwoIdenticalDimensions();
            readMatrixWithInstructions(matrixA,"A",d.row,d.col);
            readMatrixWithInstructions(matrixB,"B",d.row,d.col);

            printf("\nThe Sum of matrixA + matrixB is : \n");
            matrixAddSub(matrixA, matrixB, d.row,d.col, sub);

            break;

        case 3:

            printf("\nEnter the scalar: ");
            scanf("%d", &scalar);
            printf("\nThe scalar is: %d ", scalar);

            d = readTwoIdenticalDimensions();
            readMatrixWithInstructions(matrixA,"A",d.row,d.col);

            printf("\nThe scalar multiplication between matrixA * %d is: \n", scalar);
            matrixScalarMultiply(matrixA, scalar,d.row,d.col);

            break;

        case 4:
            //when mulotiplying arrays matrixA colum # has to equal matrixB row #
            readDimentions(&da, "A");
            readDimentions(&db, "B");

            // Column of first matrix should be equal to column of second matrix and
            while (da.col != db.row)
            {
                printf("\n\nError! column of first matrix not equal to row of second.\n\n");
                readDimentions(&da, "A");
                readDimentions(&db, "B");
            }
            readMatrixWithInstructions(matrixA,"A",da.row,da.col);
            readMatrixWithInstructions(matrixB,"B",db.row,db.col);

            //multiplyng arrays
            matrixMultiply(matrixA, matrixB, da.row, da.col, db.col);

            break;

        default:
            printf("\nIncorrect option! Please choose a number 1-4.");
            break;
        }

        printf("\n\nDo you want to calculate again? Y/N\n");
        scanf(" %c", &again);
        again = toupper(again);
    }
    printf("\n\nGoodbye!\n\n");

    return 0;
}
struct dimensions readTwoIdenticalDimensions(){
    struct dimensions da,db;
    readDimentions(&da, "A");
    readDimentions(&db, "B");

    while ((da.row != db.row) || (da.col != db.col)){ // fixed from && to ||
        printf("\nMatrices must be the same size\n");

        readDimentions(&da, "A");
        readDimentions(&db, "B");
    }
    return da;
}

void readDimentions(struct dimensions* d, char name[]){
    printf("\nEnter the #rows and #cols for matrix %s: ",name);
    scanf("%d%d",&( d->row ), &( d->col ));
}

void readMatrixWithInstructions(int matrix[10][10], char name[],int rows,int cols){
    printf("\n\tEnter elements of Matrix %s a %d x %d matrix.\n",name, rows, cols); // with the %d we remember the user the dimentions of the array
    readMatrix(matrix, rows, cols);
    printf("\n\t\tMatrix %s\n\n",name);
    printMatrix(matrix, rows, cols);
}

//User Defined Function Definition
void readMatrix(int array[10][10], int rows, int colums){
    int i, j;
    for (i = 0; i < rows; i++){
        printf("\t%d entries for row %d: ", colums, i + 1);
        for (j = 0; j < colums; j++){
            scanf("%d", &array[i][j]);
        }
    }

    return;
}

void printMatrix(int array[10][10], int rows, int colums){
    int i, j;

    for (i = 0; i < rows; i++) {
        for (j = 0; j < colums; j++){
            printf("\t%d", array[i][j]);
        }
        printf("\n");
    }
}


void matrixAddSub(int arrayone[10][10], int arraytwo[10][10], int rows, int colums, int mul){
    int i, j;
    int sumM[10][10];
    int scaM[10][10];
    for (i = 0; i < rows; i++){
        for (j = 0; j < colums; j++){
            scaM[i][j] = mul * arraytwo[i][j];
        }
    }


    for (i = 0; i < rows; i++){
        for (j = 0; j < colums; j++){
            sumM[i][j] = arrayone[i][j] + scaM[i][j];
            printf("\t%d", sumM[i][j]);
        }
        printf("\n");
    }
}


void matrixScalarMultiply(int array[10][10], int scalar, int rows, int colums){
    int i, j;
    int scaM[10][10];
    for (i = 0; i < rows; i++){
        for (j = 0; j < colums; j++){
            scaM[i][j] = scalar * array[i][j];
            printf("%d\t", scaM[i][j]);
        }
        printf("\n");
    }

}

void matrixMultiply(int arrayone[10][10], int arraytwo[10][10], int rowsA, int columsA,int columsB){
    int i, j, k;
    int mulM[10][10];

    // Initializing all elements of result matrix to 0
    for (i = 0; i<rowsA; ++i)
        for (j = 0; j<columsB; ++j)
        {
            mulM[i][j] = 0;
        }

    // Multiplying matrices a and b and
    // storing result in result matrix
    for (i = 0; i<rowsA; ++i)
        for (j = 0; j<columsB; ++j)
            for (k = 0; k<columsA; ++k)
            {
                mulM[i][j] += arrayone[i][k] * arraytwo[k][j];
            }
    printf("\nOutput Matrix:\n");
    for (i = 0; i<rowsA; ++i)
        for (j = 0; j<columsB; ++j)
        {
            printf("\t%d ", mulM[i][j]);
            if (j == columsB - 1)
                printf("\n\n");
        }
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ This is good advice. I was just about to type up the same thing! Great first answer. \$\endgroup\$ – user1118321 Mar 25 '18 at 1:04
0
\$\begingroup\$

This is a reasonably good first start. I think it could be improved a little.

Functions

You have a lot of redundant code, and a lot of different types of code mixed together. Your programs will be easier to understand and maintain, and possibly smaller and faster, if you break them up into functions that handle the various tasks you need to do.

Others have mentioned getting the input from the user as a function you could make. That's a good idea. It looks to me like you have several places where you are outputting the results, too. This code should be separate from the code that calculates the results. This is often called separation of concerns, or keeping your business logic separate from your display logic. Furthermore, if you've made a mistake in the display code, you only have to fix it in one place. If you have the same code in multiple places and you discover an error in it, you now have to check each place you have duplicate code to see if you made the same error. It quickly becomes difficult to maintain.

Formatting

Another thing that makes code easier to read, change, and fix is to format it with those goals in mind. While C allows you to omit the braces when the body of a for loop or if class is a single line, this is asking for trouble. Later, when the code needs to be modified, it's easy to forget that there are no braces, add a line with the same indentation and have code that doesn't do what you think it does. For that reason, I recommend always adding braces around every for, if, do, and while body, even if it's only a single line.

Also, your brace style is inconsistent. In some cases you put the opening brace on the same line as the function name or for statement, and in other cases it's below it. It doesn't matter which you use, but it's a good idea to be consistent and use the same one in all cases.

It's best to create variables as close to their first use as you can. This makes it easier to see and understand the lifetime of a variable. It can also help the compiler to optimize things if it knows it doesn't need to use all 15 variables at once in a given function.

Avoid Magic Numbers

You have the number 10 in the code something like 39 times. If you ever decide you need to change the maximum size to something else, you have to change it in 39 places. If you used a named constant, you'd only have to change it one place and recompile. You could put this at the top of the file:

const size_t kMaxRowSize = 10;
const size_t kMaxColSize = 10;

Then declare your variables like this:

int array[kMaxRowSize][kMaxColSize];

A good rule of thumb is that any time you have a bare number other than 0 or 1 in your code, you probably want to make a named constant for it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.