Always the same output in C

Question

I wonder why the last printf of my program is always the same? I made it in C and I am beginner. The purpose of the program is to check if a number is palindromic or not. But when it comes time to print the result, in all executions of the program the result is always that the number is not palindromic. How do I fix it?

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 = '\0';
    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");
    }

}

Answer 1

There are two main problems with the above code.

1. Its implementation of strrev simultaneously inverts the received String as parameter in-place and returns a pointer to that string.

   Thus the line strcpy(reversedNum, strrev(stringOfTheNum)); doesn’t exactly do what you expect. stringOfTheNum is reversed first and then copied to reversedNum. The result is that stringOfTheNum will always be equal to reversedNum (both reversed). You can work around this problem by copying the String first and then inverting:

   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
   

2. The second problem is that strcmp returns 0 when two strings are equal. In C 0 is equivalent to false, then your condition is reversed. The correct one would be:

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

   Or else:

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

See Working on Ideone

---

There are other possible improvements to the code.

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

Some examples:

1. Any part of the code that computes the amount 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 ('\0')
   

   However, bufferSize is not even necessary; just use a fixed size buffer large enough to contain the maximum value of digits an integer can have on its platform + two extra characters; one for a possible negative 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-digit), so your program can store all possible input values using strings of size equal to 12. Since memory is not such a scarce resource for the vast majority of systems, you could even use a grossly overrated buffer (and. g., char[100]) and not worry about the subject.

2. The function itoa in your case may also be replaced by:

   snprintf(stringOfTheNum, bufferSize, "%d", numberWantToCheck);
   

3. The implementation of strrev above (found here) is very clever, but uses tricks involving pointer arithmetic + XOR operations that are clearly not the work of a beginner. In fact, this is the kind of "smart" code, hard to read, that I would replace with a trivial loop + temporary variable in 99.9% of the cases. The compiler does a very good job optimizing this type of thing; I’m not saying that this is necessarily the case with this code, but I’ve found many "optimizations" like this that end up worsening the performance of the application. If any of my students gave me a role as part of an exercise I would very 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 wouldn’t even need to convert your integer to a String, see that example. Unused arrays of characters the code in question becomes much simpler. And of breaking you need not worry about buffer overflow and other problems of this kind.