2
\$\begingroup\$

I just finished a iterative version of Hanoi Tower.

This is how the algorithm works:

enter image description here

I'm trying to find a method to delete the goto instruction. Any tips?

/**
 * definition of all function: 
 * 
 *  print : just print the state of the array
 *  populate: just fill the array with 0
 *  mov_pl: move the pile from to 
 *  CHK_POS_FROM : check from where come the disk
 *  CHK_POS_TO: check where the disk will go
 *  CHK_STK: check the status of the pile
 **/


    #include <stdio.h>
    #include <math.h>
    #include <stdlib.h>

    // Number if pile
    #define DM_STCK 3

    // Number of disk 
    #define DM_CLMN 10


    void print(int STCK[][DM_STCK], int DM_S);  
    void MOV_PL(int STCK[][DM_STCK], int FROM, int TO, size_t DM_S); 
    int CHK_POS_FROM(int STCK[][DM_STCK], int N_PL, size_t DM_S);
    int CHK_POS_TO(int STCK[][DM_STCK], int N_PL, size_t DM_S);
    int CHK_STK(int STCK[][DM_STCK], int N_PL, size_t DM_S);
    void populate(int STCK[][DM_STCK], int DM_S);

    int main(int argc, char *argv[]){

    // name of each pule

        int PL_1 = 0;
        int PL_2 = 1;
        int PL_3 = 2;

    // check if the disk are odd or even

        if(DM_CLMN % 2 == 0){
            PL_2 = 2;
            PL_3 = 1;
        }


        int stack[DM_CLMN][DM_STCK];

        populate(stack, DM_CLMN);

        long long mosse = (long long)pow(2,DM_CLMN) - 1;

        int RS_1 = 0;
        int RS_2 = 0;

        puts("### START ###");

        print(stack, DM_STCK);

        for(int cnt = 0; cnt < mosse; cnt++){

            RS_1 = 0;
            RS_2 = 0;

            printf("MOSSA NUMERO: %d\n", 1 + cnt);

            if(cnt % 3 == 0){

                    RS_1 = CHK_STK(stack, PL_1, DM_CLMN);
                    RS_2 = CHK_STK(stack, PL_3, DM_CLMN);

                    if(RS_1 > RS_2){
                        if(RS_2 == 0){
                        MOV_PL(stack, PL_1, PL_3, DM_CLMN);
                        print(stack, DM_STCK);

                        //printf("A --> B");
                        } else {
                            ODINO_0:
                            MOV_PL(stack, PL_3, PL_1, DM_CLMN);
                            print(stack, DM_STCK);
                            continue;
                        //printf("B --> A");
                        }
                    } if(RS_1 < RS_2) {
                        if(RS_1 == 0){
    //                      Odino perdonami per questa istruzione
                            goto ODINO_0;
                        }
                        MOV_PL(stack, PL_1, PL_3, DM_CLMN);
                        print(stack, DM_STCK);

                    }

                }

            if(cnt % 3 == 1){

                    RS_1 = CHK_STK(stack, PL_1, DM_CLMN);
                    RS_2 = CHK_STK(stack, PL_2, DM_CLMN);

                    if(RS_1 > RS_2){
                        if(RS_2 == 0){
                        MOV_PL(stack, PL_1, PL_2, DM_CLMN);
                        print(stack, DM_STCK);

                        //printf("A --> B");
                        } else {
    //                  Che odino sia con me
                        ODINO_1:
                        MOV_PL(stack, PL_2, PL_1, DM_CLMN);
                        print(stack, DM_STCK);

                        continue;
                        //printf("B --> A");
                        }
                    } else {
                        if(RS_1 == 0){
    //                      Odino perdonami per questa istruzione
                            goto ODINO_1;
                        }
                        MOV_PL(stack, PL_1, PL_2, DM_CLMN);
                        print(stack, DM_STCK);

                    }
            }

            if(cnt % 3 == 2){

                RS_1 = CHK_STK(stack, PL_2, DM_CLMN);
                RS_2 = CHK_STK(stack, PL_3, DM_CLMN);

                    if(RS_1 > RS_2){
                        if(RS_2 == 0){
                        MOV_PL(stack, PL_2, PL_3, DM_CLMN);
                        print(stack, DM_STCK);

                        //printf("A --> B");
                        } else {
                        ODINO_2:
                        MOV_PL(stack, PL_3, PL_2, DM_CLMN);
                        print(stack, DM_STCK);

                        continue;
                        //printf("B --> A");
                        }
                    } else {
                        if(RS_1 == 0){
    //                      Odino perdonami per questa istruzione
                            goto ODINO_2;
                        }
                        MOV_PL(stack, PL_2, PL_3, DM_CLMN);
                        print(stack, DM_STCK);

                    }
            }

        }

        puts("### END ###");        


        return 0;
    }

    void populate(int STCK[][DM_STCK], int DM_S){

        for(int colonna = 0; colonna < DM_S; colonna++){
            for(int riga = 0; riga < DM_STCK; riga++){
                STCK[colonna][riga] = 0;
            }
        }

        for(int cnt_1 = 0; cnt_1 < DM_S; cnt_1++){
            STCK[cnt_1][0] = cnt_1 + 1;
        }

    }

    // Stampa le 3 torri

    void print(int STCK[][DM_STCK], int DM_S){
        for(int colonne = 0; colonne < DM_CLMN; colonne++){
            for(int righe = 0; righe < DM_STCK; righe++){

                if(STCK[colonne][righe] == 0){
                    printf("|   |");
                }else{
                    printf("| %d |", STCK[colonne][righe]);
                }
            }
            puts("\n");
        }

        puts("\n");
    }

    // Controlla in che direzione muovere il paletto se -> o <-
    // check wich direction to move the pile if left or right

    void MOV_PL(int STCK[][DM_STCK], int FROM, int TO, size_t DM_S){

        int POS_1 = CHK_POS_FROM(STCK, FROM, DM_S);
        int POS_2 = CHK_POS_TO(STCK, TO,DM_S);

        int TMP = STCK[POS_2][TO];
        STCK[POS_2][TO] = STCK[POS_1][FROM];
        STCK[POS_1][FROM] = TMP;

    }

    // Estrae la posizione del numero da posizione
    // Look at the disk position in the array

    int CHK_POS_FROM(int STCK[][DM_STCK], int N_PL, size_t DM_S){

        int POS = DM_S - 1;

        for(int cnt_1 = 0; cnt_1 < DM_S; cnt_1++){
            if(STCK[cnt_1][N_PL] != 0){
               POS = cnt_1;
               break;
          }
        }


        return POS;

    }

    /* 
         Estrae la posizione di dove posizione il numero da posizione e 
        controlla che non sia uno zero, se non è uno zero allora è pos - 1 altrimenti 
        solo pos

        Extract the position, so we can tell in wich position is and check if 
        it's  0, if it's not a 0 pos = pos - 1 otherwise is just pos
    */


    int CHK_POS_TO(int STCK[][DM_STCK], int N_PL, size_t DM_S){

        int POS = DM_S - 1;

        for(int cnt_1 = 0; cnt_1 < DM_S; cnt_1++){
            if(STCK[cnt_1][N_PL] != 0){
               POS = cnt_1 - 1;
               break;
          }
        }


        return POS;

    }

    /*

        Estra il primo anello dallo stack e lo confronta con l'anello del l'altro stack
        per decidere in quale verso spostare l'anello.

        Extract the first disk from the stack and compares with the other disk
        from the pile, to decide in wich way to move the disk left or right

    */ 

    int CHK_STK(int STCK[][DM_STCK], int N_PL, size_t DM_S){

        int FLG = 0;

        for(int cnt = 0; cnt < DM_S; cnt++){
            if(STCK[cnt][N_PL] != 0){
               FLG = STCK[cnt][N_PL];
            break;
          }
        }

     return FLG;

    }
\$\endgroup\$
  • 1
    \$\begingroup\$ the posted code produces 9 warnings, many of which are critical. When compiling, always enable the warnings, then fix those warnings. (for gcc, at a minimum use: -Wall -Wextra -Wconversion -pedantic -std=gnu11 ) \$\endgroup\$ – user3629249 Mar 5 '18 at 10:59
3
\$\begingroup\$

for ease of readability and understanding:

  1. consistently indent the code. indent after every opening brace '{'. unindent before every closing brace '}'. Suggest each indent level be 4 spaces.
  2. use meaningful variable and parameter names. Names should indicate content or usage (or better, both)
  3. separate code blocks ( for if else while do...while switch case default ) by a single blank line.
  4. separate functions by 2 or 3 blank lines (be consistent)
  5. it is best to not introduce random blank line
  6. is is best to not introduce random indenting
  7. follow the axiom: only one statement per line and (at most) one variable declaration per statement.
  8. Treat the closing brace '}' as a separate statement.
  9. regarding: int main(int argc, char *argv[]) since neither parameter is being used, suggest using: int main( void )
  10. regarding: void print(int STCK[][DM_STCK], int DM_S) since 'int DM_S' is not used, either eliminate that parameter or the first line in the body of the function should be: (void)DM_S;
  11. it is a bad programming practice to include header files those contents are not being used. I.E. suggest removing the statement: #include <stdlib.h>
  12. strongly suggest removing the label: ODINO_0 and replacing any 'goto' to ODINO_0 with the three lines of code following the label. Similar considerations exist for ODINO_1 and ODINO_2
  13. since DM_S is being passed as a size_t, any other variables used with it should also be declared as size_t, not int
  14. for ease of readability and understanding: DO NOT use all CAPS for variable, parameter, and function names. (all CAPS is considered 'shouting') It is appropriate to use all caps for macro names.
  15. for ease of readability and understanding: insert a (reasonable) space: inside parens, inside square brackets, after commas, after semicolons, around C operators
\$\endgroup\$
  • \$\begingroup\$ i will definetly try to improve the quality of the code, you're right it'was a mistake to use all this CAPS. Thanks for your feed much appreciated ! \$\endgroup\$ – Robert Mar 6 '18 at 1:39
2
\$\begingroup\$

Your code:

if(RS_1 > RS_2) {
   if(RS_2 == 0) {
      // do A
   } else {
      ODINO_0:
      // do B
   }
} if(RS_1 < RS_2) {
   if(RS_1 == 0) {
      goto ODINO_0;
   }
   // do C
}

can be rewritten as:

if(RS_2 == 0) {
   // do A
} else if(RS_1 > RS_2 || RS_1 == 0) {
   // do B
} else if(RS_1 < RS_2) {
   // do C
}

under the assumption that these two values are never negative. If they can be negative there are some additional tests necessary.

I have nothing against goto, sometimes it's clean and clear. In this case, however, it is confusing, it takes a while to figure out where it jumps to.

Regarding naming: please use longer variable and function names. 3-letter all-caps names look like Fortran 66 code. It is hard to read your code because variable names are meaningless, and there is no comments explaining their use. Good variable names make code easy to read (and there is no need for comments if they are good enough). Plus, modern IDEs do name completion, you don't even have to type the full names any more. :D

\$\endgroup\$
  • \$\begingroup\$ You're right, i will try it as soon as possible. I need to improve my logic operation skill, any tips or source from where to train? \$\endgroup\$ – Robert Mar 6 '18 at 1:40

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.