What is refactoring, what is it for and when to apply it?

[Note: C examples are used, but technique agnostic]

Refactoring a code means transforming its source code without altering its results. This is done to simplify it, reduce duplication, remove dead code, and make it more understandable, thus easier to maintain and update.

It is applied to working code (correct logic), and proceeds iteratively, applying one transformation after another.

In this example, since we are trying to make the game multidimensional, we start by replacing all mentions of the size of the game with a constant specified by a #define. We might also want to give names to the characters used to represent a cell that is either empty or occupied by a player. We do that by declaring a enumwhich gives us the range of possible values ​​for the variable.

#define DIM 3
enum valor_celda {VACIA = '+', PLAYER_1 = 'X', PLAYER_2 = '0'};

We then check the source by changing all the '3's to DIMand the constants ' ', 'x' y 'o'to the respective names EMPTY, PLAYER_1 and PLAYER_2

The following will normally break the code into small functions. For that, the code is traversed identifying pieces that perform specific functions and extracting it to functions.

In the case shown, we have this piece

for(i=0; i<DIM; i++){
    for(j=0; j<DIM; j++){
        matriz[i][j]=VACIA;
        printf(" %c", matriz[i][j]);
     }
    printf("\n");
}

and I'm going to move it to a separate function. I'm also going to remove the printf, because you shouldn't mix operation (zero the array) with I/O (print the array).

void matriz_zero(char m[DIM][DIM]) {
    //  Inicializa las celdas con el caracter VACIA
    for (int i = 0; i < DIM; i++) {
        for (int j = 0; j < DIM; j++) {
            m[i][j] = VACIA;
        }
    }
}

Notice that I pass the array as a parameter instead of using a global variable. This gives me versatility, since the program could use more than one matrix without the need for changes. Avoid global variables.

And to print the matrix, which has to be done in several parts, I define a print function:

void imprimir_matriz(char m[DIM][DIM]) {
    //  Imprime la matriz.
    for (int i = 0; i < DIM; i++) {
        for (int j = 0; j < DIM; j++) {
            printf(" %c", m[i][j]);
        }
        printf("\n");
    }
}

Now the program is a bit simpler. The beginning of the original looks like this:

int main(){
    char matriz[DIM][DIM], opc;
    int i, j;

    matriz_zero(matriz);
    imprimir_matriz(matriz);
    int fila, col, ganador=0, turno=1;

    //para jugador 1
    do{

If we look at what follows, we see that it is about alternating between two players and clearly all the code is duplicated:

        if(turno%2==1){
            do{
                scanf("%d", &fila);
                scanf("%d", &col);
                //ciclo para cuando el usuario ingrese coordenadas invalidas
                if(matriz[fila][col] == PLAYER_1 || matriz[fila][col] == PLAYER_2 || fila > DIM-1 || col > DIM-1){
                }
            } while(matriz[fila][col] == PLAYER_1 || matriz[fila][col] == PLAYER_2 || fila > DIM-1 || col > DIM-1);
            
            matriz[fila][col]=PLAYER_1;
            
            for(i=0; i<DIM; i++){
                for(j=0; j<DIM; j++){
                    printf(" %c", matriz[i][j]);
                }
                printf("\n");
            }
            turno++;
            

There are two separate actions here:

• Request the coordinates of the cell to fill, check that it is empty and fill it.
• Print it.

so naturally I will divide the chunk into two parts. Since I already have the print ready, I only have to enter the coordinates and populate the cell:

int pedir_celda(char m[DIM][DIM], char marca) {
    //  Pide una celda, valida que este vacia, la llena y revisa si es un ganador.
    int fila, col;
    while (1) {
        printf("--- Jugador %c ---\n", marca);
        printf("> fila: ");
        scanf("%d", &fila);
        printf("> columna: ");
        scanf("%d", &col);

        if (fila >= 0 && fila < DIM && col >= 0 && col < DIM) {
            if (m[fila][col] == VACIA) {
                m[fila][col] = marca;
                imprimir_matriz(m);
                break;
            } else {
                printf("Celda está ocupada\n");
            }
        } else {
            printf("Coordenadas fuera de rango\n");
        }
    }

    return revisar_ganador(m, fila, col);
}

This function is general. It receives in mark the character that identifies the player ('x' or '0'), so it works for both cases. After populating the cell, it prints the matrix so the player can see the result of their move.

Here we made a small change in the logic: the function calls revisar_ganador, so when it returns it already knows if the current player has won the game.

With that, mainit boils down to:

int main() {
    int ganador = 0;
    int turno = 1;
    char matriz[DIM][DIM];

    matriz_zero(matriz);
    imprimir_matriz(matriz);

    while (!ganador) {
        if (turno) {
            ganador = pedir_celda(matriz, PLAYER_1);
        } else {
            ganador = pedir_celda(matriz, PLAYER_2);
        }
        turno = !turno;
    }
    printf("Ganaste!!!");
    return 0;
}

This is much more understandable. In a few lines, the general logic of the application is presented, which is structured as a collection of short, simple functions with a single purpose.

It remains for us to account for these lines:

    if(matriz[0][0] == 'x' && matriz[0][0] == matriz[0][1] && matriz[0][0] == matriz[0][2]
        || matriz[1][0] == 'x' && matriz[1][0] == matriz[1][1] && matriz[1][0] == matriz[1][2]
            || matriz[2][0] == 'x' && matriz[2][0] == matriz[2][1] && matriz[2][0] == matriz[2][2]
                
                || matriz[0][0] == 'x' && matriz[0][0] == matriz[1][0] && matriz[0][0] == matriz[2][0]
                    || matriz[0][1] == 'x' && matriz[0][1] == matriz[1][1] && matriz[0][1] == matriz[2][1]
                        || matriz[0][2] == 'x' && matriz[0][2] == matriz[1][2] && matriz[0][2] == matriz[2][2]
                            
                            || matriz[0][0] == 'x' && matriz[0][0] == matriz[1][1] && matriz[0][0] == matriz[2][2]
                                || matriz[0][2] == 'x' && matriz[0][2] == matriz[1][1] && matriz[0][2] == matriz[2][0]){
        ganador=1;
        printf("1\n");
    }

Clearly the intent is to determine if a row, column, or diagonal is filled, but the code is not to transform it to NxN. You have to implement a general solution, which is not derived from the original code.

int revisar_diagonales(char m[DIM][DIM]) {
    //  Revisa si alguna de las dos diagonales maximas esta totalmente
    //  llena con la misma marca.
    //  Retorna True en tal caso.    
    int en_linea = 0;
    char marca = m[0][0];
    int count = 0;
    for (int fila = 0; fila < DIM; fila++) {
        for (int col = 0; col < DIM; col++) {
            if (m[fila][col] == marca)
                count++;
        }
    }
    if (count != DIM) {
        marca = m[0][DIM - 1];
        count = 0;
        for (int fila = 0; fila < DIM; fila++) {
            for (int col = DIM - 1; col >= 0; col--) {
                if (m[fila][col] == marca)
                    count++;
            }
        }
    }
    return (count == DIM);
}

int revisar_ganador(char m[DIM][DIM], int fila, int col) {
    //  Revisa el jugador en la celda (fila, col) ha completado
    //  las celdas requeridas
    
    // Todas las celdas deben tener el mismo valor que (fila, col)
    char marca = m[fila][col];
    int i;

    //  Recorrer la misma fila contando las celdas iguales.
    for (i = 0; i < DIM; i++) {
        if (m[fila][i] != marca)
            break;
    }
    if (i == DIM) {
        return 1;
    }

    //  Recorrer la misma columna contando las celdas iguales.
    for (i = 0; i < DIM; i++) {
        if (m[fila][i] != marca)
            break;
    }
    if (i == DIM) {
        return 1;
    }

    return revisar_diagonales(m);
}

In this example we have not detected dead code. Nothing is left over in this version.

That is refactoring. When you have problems debugging code (yours or someone else's), apply refactoring to make it easier to understand.