tags:

views:

144

answers:

4

I'm writing some useful functions in C. One of them is isPalindrome().

I figured to determine if a number is a palindrome or not, I should...

  • get all digits in an array
  • iterate through with two indexes - start one at 0 and one to the array count
  • increment/decrement the indexes whilst subscripting the array whilst they match and if the array count gets to 0 we have a palindrome (i.e. finishing going through all digits).

I came up with...

int isPalindrome(int num) {

    int places[100];
    int i = 0;
    while (num > 0) {
        places[i++] = num % 10; 
        num /= 10;
    }

    int j = 0;
    while (i >= 0 && places[j++] == places[--i]) {
    }
    return i == -1;

}

Is this generally how it is done?

I'm learning C by myself, and although I can tell when my code compiles and doesn't take all day to work something out, I don't have any expert eyes to tell me if I'm on the right track.

So, any improvements or suggestions on my code?

Thanks very much!

+5  A: 

You only have to loop while i > j. Once i <= j, you are just checking all the characters a second time.

James McNellis
+1 but when i==j, it's not a second time check
pmg
@pmg: when i == j the comparison will always be true.
JeremyP
@Jeremy: yes, but it's the first time *that* comparison is done.
pmg
+2  A: 

Although using inline ++ and -- operators in the following might seem clever:

while (i >= 0 && places[j++] == places[--i]) { 
} 

your code will be easier to read if you put those inside the loop body:

while (i >= 0 && places[j] == places[i-1]) { 
    j++;
    i--;
} 

This way, the reader of the code won't have to think about the possible side effects of changing the values of i and j within the conditional test. There will probably be no measurable effect on the speed of the compiled code (although, if performance is important to this function, you should check with your compiler).

Also, you've got a bug where you will access places[-1] if i == 0.

Greg Hewgill
+1  A: 

I'd just use sprintf to "convert the string to digits":

char places[100];
sprintf(places, "%i", num);
i = strlen(places);
sth
Thanks, this looks useful. Unfortunately I didn't know about this when I wrote the code.
alex
+1  A: 

In java

static boolean isPalindrome(String p) {
    return p.equals(new StringBuilder(p).reverse().toString());
}

In c++ and c

int IsPalindrome(char *string) {
    int bottom = 0, top;

    top = strlen(string) - 1;
    while(bottom < top && string[bottom] == string[top]) {
        ++bottom;
        --top;
    }
    return (bottom >= top ? 1:0);
}

Note, You need to write itoa function, if you need to do this for a number input. Or use ( link ).

Thats how it is generally done. This would also work for all bases and not only 10.

Margus
while we're on the topic of other languages ... in scala -> def isPalindrome(p: String) = p.equals(p.reverse)
Synesso
My possible palindrome is not a string, it is an 'int'.
alex
If you mean shorter is better then in J it is: `((-:|.)@:":)"0`
Margus
@alex Yes, I considered that as bug.
Margus
In C++ I'd use std::string rather than char* and then either use the begin/rbegin to get a forward and reverse iterator, or make a reverse_copy of the string and then compare as you did for the Java solution.
GrahamS