Always output the same in C

1

I wonder why the last printf of my program is always the same? I made it in C and I'm a beginner. The purpose of the program is to verify that a is palindrome or not. But when it comes time to print the result, in every run of the program the result is always that the number is not palindrome. How do I resolve this?

Code:

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

char *strrev(char *str)
{
     char *p1, *p2;

  if (! str || ! *str)
        return str;
  for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
  {
        *p1 ^= *p2;
        *p2 ^= *p1;
        *p1 ^= *p2;
  }
  return str;
}

char* itoa(int i, char b[]){
    char const digit[] = "0123456789";
    char* p = b;
    if(i<0){
        *p++ = '-';
        i *= -1;
    }
    int shifter = i;
    do{ //Move to where representation ends
        ++p;
    shifter = shifter/10;
    }while(shifter);
    *p = '
#include<stdio.h>
#include<stdlib.h>
#include<string.h>

char *strrev(char *str)
{
     char *p1, *p2;

  if (! str || ! *str)
        return str;
  for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
  {
        *p1 ^= *p2;
        *p2 ^= *p1;
        *p1 ^= *p2;
  }
  return str;
}

char* itoa(int i, char b[]){
    char const digit[] = "0123456789";
    char* p = b;
    if(i<0){
        *p++ = '-';
        i *= -1;
    }
    int shifter = i;
    do{ //Move to where representation ends
        ++p;
    shifter = shifter/10;
    }while(shifter);
    *p = '%pre%';
    do{ //Move back, inserting digits as u go
        *--p = digit[i%10];
        i = i/10;
    }while(i);

}

int main(void){
    int contaDigit = 0, valor;
    int numberWantToCheck;
    printf("What's the number you want to check?\n");
    scanf("%d", &numberWantToCheck);
    valor = numberWantToCheck;
    do{
         contaDigit += 1;
         valor /= 10;
    }while(valor != 0);
    char stringOfTheNum[contaDigit];
    itoa(numberWantToCheck, stringOfTheNum);
    char reversedNum[contaDigit];
    strcpy(reversedNum, strrev(stringOfTheNum));
    if(strcmp(reversedNum , stringOfTheNum)){
        printf("The number is palindrome.\n");
    }else{
        printf("The number is not palindrome.\n");
    }

}
'; do{ //Move back, inserting digits as u go *--p = digit[i%10]; i = i/10; }while(i); } int main(void){ int contaDigit = 0, valor; int numberWantToCheck; printf("What's the number you want to check?\n"); scanf("%d", &numberWantToCheck); valor = numberWantToCheck; do{ contaDigit += 1; valor /= 10; }while(valor != 0); char stringOfTheNum[contaDigit]; itoa(numberWantToCheck, stringOfTheNum); char reversedNum[contaDigit]; strcpy(reversedNum, strrev(stringOfTheNum)); if(strcmp(reversedNum , stringOfTheNum)){ printf("The number is palindrome.\n"); }else{ printf("The number is not palindrome.\n"); } }
    
asked by anonymous 20.12.2018 / 20:38

1 answer

5

There are two major issues with the above code.

  • Your implementation of strrev simultaneously inverts the received String as an in-place parameter and returns a pointer to that String.

    This way the strcpy(reversedNum, strrev(stringOfTheNum)); line does not do exactly what you expect. stringOfTheNum is reversed first and then copied to reversedNum . The result is that stringOfTheNum will always equal reversedNum (both inverted). You can work around this by copying the String first and then reversing:

    char stringOfTheNum[contaDigit];
    char reversedNum[contaDigit];
    itoa(numberWantToCheck, stringOfTheNum); // converte o numero original para String
    strcpy(reversedNum, stringOfTheNum); // copia a string para reversedNum
    strrev(reversedNum); // inverte reversedNum
    
  • The second problem is that strcmp returns 0 when two strings are equal. In C 0 is equivalent to false , so its condition is inverted. The correct would be:

    if(strcmp(reversedNum , stringOfTheNum) == 0) {
        printf("The number is palindrome.\n");
    } else {
        printf("The number is not palindrome.\n");
    }
    

    Or:

    if(strcmp(reversedNum , stringOfTheNum)) {
        printf("The number is not palindrome.\n");
    } else {
       printf("The number is palindrome.\n");
    }
    
  • See Running on Ideone

    There are other possible improvements in the code.

    One of the problems, in my view, is that you are reimplementing non-standard functions in C as itoa and strrev instead of using standard library functions.

    Some examples:

  • Any part of the code that computes the number of characters needed for the buffer can be replaced by:

    int bufferSize = snprintf(NULL, 0, "%d", numberWantToCheck) + 1;
    // O +1 está prevendo que a linha será terminada em NUL ('
    snprintf(stringOfTheNum, bufferSize, "%d", numberWantToCheck);
    
    ')

    However, bufferSize is not even necessary; just use a fixed-size buffer large enough to hold the maximum number of digits that an integer can have on your platform + two extra characters; one for a possible minus sign + one for the string terminator. For example: For 32-bit integers the maximum value of an integer would be +2,147,483,647 (10 digits), so your program can store all possible input values using Strings of size equal to 12. As memory is not a resource so much scarce for most systems, you could even use a grossly overrated buffer (eg, char[100] ) and not worry about it.

  • The itoa function in your case can also be replaced by:

    char stringOfTheNum[contaDigit];
    char reversedNum[contaDigit];
    itoa(numberWantToCheck, stringOfTheNum); // converte o numero original para String
    strcpy(reversedNum, stringOfTheNum); // copia a string para reversedNum
    strrev(reversedNum); // inverte reversedNum
    
  • The implementation of strrev above (found here ) is very clever, but uses tricks involving arithmetic of pointers + XOR operations that clearly are not the work of a beginner. In fact, this is the "smart" type of code, hard to read, that I would replace with a trivial loop + temporary variable in 99.9% of cases. The compiler does a pretty good job optimizing this sort of thing; I do not say that this is necessarily the case of this code, but I've already found many "optimizations" like this that end up worsening the performance of the application. If any of my students gave me a function as well as part of an exercise I would most likely ask him to explain to me how this code works, as well as the motivation for such "optimization".

  • Finally, since your input is always a number, you would not even have to convert your integer to a String, see this example . Without using arrays of characters the code in question becomes much simpler. And you do not have to worry about buffer overflow and other issues of that kind.

        
    20.12.2018 / 23:48