3

I am accepting a string as a command line argument. I want to check whether the inputted string is a palindrome or not and print the result. I have written the following code. But its displaying the result 'not palindrome' for all inputs.

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

int main(int argc, char argv[20]) {
    int i;
    int l = strlen(argv);    
    char str[20];
    bzero(str, 20);

    for(i=0; i<l; i++)
    {
        str[i] = argv[i+2];
    } 
    int flag;
    int len = strlen(str);
    for(i=0; i< len/2; i++)
    {
        if(str[i] == str[len - (i+2)])
        {
            flag = 0;
        }
        else
        {
            flag = 1;
            break;
        }
    }

    if(flag == 0)
        printf("Palindrome\n");
    else
        printf("Not a palindrome\n");
}
abelenky
  • 63,815
  • 23
  • 109
  • 159
Khushboo
  • 285
  • 3
  • 5
  • 12
  • I'm not able to format the code here. Can someone please help me with that too along with the programming issue? How do I print every line of code on a different line? – Khushboo Aug 12 '10 at 19:07
  • To format your code, indent with four spaces, or use the 101010 button. It's not readable the way it is now. – Thomas Aug 12 '10 at 19:07
  • @user417316, paste your code into the text box from whatever editor you're using, select it all, and then pick the code formatting button (it looks like little 1s and 0s). – Carl Norum Aug 12 '10 at 19:08
  • Not an answer to your question, but there is no need to reset `flag` on every successful iteration. Just initialize it and only change it on failure. Also you have the wrong type for `argv` which should be `char**`; this could be causing almost any kind of havoc. – dmckee --- ex-moderator kitten Aug 12 '10 at 19:11
  • @Thomas: Thank you !! It helped. @Carl Norum: Thank you for your help !! @dmckee: Thank you ! – Khushboo Aug 12 '10 at 19:16
  • @user417316, if this is homework, please tag properly... – Bruno Brant Aug 12 '10 at 19:28

6 Answers6

5

You could do it in a K&R-style by having two offset iterators in a for-loop:

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

int main(int argc, char *argv[]) {
    assert(argc != 1);

    char *text = argv[1];

    int len = strlen(text);
    int is_palindrome = 1;
    int i, j;

    for(i = 0, j = len - 1; i < len / 2; i++, j--) {
        if(text[i] != text[j]) {
            is_palindrome = 0;
            break;
        }
    }

    printf("%s a palindrome.\n", is_palindrome ? "is" : "isn't");

    return(0);
}

Changes from original:

  • Changed shift(len >> 1) to division(len / 2) as tenfour suggested.
gamen
  • 987
  • 6
  • 12
  • actually I'm not quite comfortable programming with the use of pointers. I need to gain some more information and understanding of pointers. Maybe then I'll try to program with use of pointers. Is there any other way I can solve the palindrome problem? – Khushboo Aug 14 '10 at 07:24
  • optimization: `int len = strlen(text) / 2;` – tenfour Aug 14 '10 at 08:23
  • I'd like to recommend Binky's pointer fun videos: http://cslibrary.stanford.edu/104/ – gamen Aug 25 '10 at 15:46
2

Updated based on comments:

int is_palindrome(const char *s)
{
   const char *t = s + strlen(s);
   while (s<t && *s==*--t) s++;
   return s>=t;
}

And since the OP wants a version that's not so heavy on pointers:

int is_palindrome(const char *s)
{
   size_t i=0, j = strlen(s);
   while (i<j && s[i]==s[--j]) i++;
   return i>=j;
}

For reference, here's the original buggy version:

int is_palindrome(const char *s)
{
   const char *t = s + strlen(s) - 1;
   while (s<t && *s++==*t--);
   return s>=t;
}
R.. GitHub STOP HELPING ICE
  • 208,859
  • 35
  • 376
  • 711
  • 1
    I don't call this cryptic. I call it not making a meal of things. When you have a trivial task to perform, don't make it look complicated by writing so much...unless you're trying to slack off and convince your boss you've written a bunch of "lines of code". – R.. GitHub STOP HELPING ICE Aug 13 '10 at 07:26
  • 1
    Unfortunately it fails for a lot of non-palindromes like "ab". Try `while (*s == *t && s++ < t--) ;`. And there is an issue with checking the empty string. – schot Aug 13 '10 at 08:17
  • I was waiting for someone to catch the technicality of decrementing `t` before the beginning of the string when the string is empty. :-) And you're right about the other bug - that's what I get for coding in a textbox on SO. I'd change it to `for (t=s+strlen(s)-1; s – R.. GitHub STOP HELPING ICE Aug 13 '10 at 09:01
  • actually I'm not quite comfortable programming with the use of pointers. I need to gain some more information and understanding of pointers. Maybe then I'll try to program with use of pointers. Is there any other way I can solve the palindrome problem? – Khushboo Aug 14 '10 at 07:25
  • OK, I'll add a version with array indices. – R.. GitHub STOP HELPING ICE Aug 14 '10 at 07:26
1

For one thing, your signature for main is off. It should be int main(int argc, char** argv) or int main(int argc, char * argv[]). You're treating a pointer to a string as if it were a string.

When you've changed that, the string you want should be in argv[1] (since argv[0] is some representation of the program name).

David Thornley
  • 56,304
  • 9
  • 91
  • 158
1

There's a good case for using pointers rather than indexes for this:

int is_palindrome(const char *s) {
    const char *end = s + strlen(s);
    while (end > s) {
        --end;
        if (*end != *s) return 0;
        ++s;
    }
    return 1;
}

If you like short, confusing code, you can re-write that:

int is_palindrome(const char *s) {
    const char *end = s + strlen(s);
    while (end > s) if (*(--end) != *(s++)) return 0;
    return 1;
}

argv isn't a string, it's an array of strings, one for the program name and then one for each argument (usually space-separated in a command line). So to test if the first argument is a palindrome, you're interested in argv[1].

int main(int argc, char **argv) {
    if (argc != 2) {
        printf("usage: %s <string>\n", argv[0]); // or something
        return 1;
    }
    if (is_palindrome(argv[1])) {
        printf("Palindrome\n");
    } else {
        printf("Not a Palindrome\n");
    }
}
Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
0

The first loop doesn't make sense. Copying the string to another doesn't make sense.

Just do it and adjust the index:

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

int main(int argc, char **argv) {
int i;
char * str = argv[1];
int flag;

int len = strlen(str);

for(i=0; i< (len+1)/2; i++)
{

    printf("DEBUG: Comparing %c %c\n",str[i], str[len - (i+1)]);


    if(str[i] == str[len - (i+1)])
    {
        flag = 0;
    }
    else
    {
        flag = 1;
        break;
    }
}

    if(flag == 0)
    printf("Palindrome\n");
else
    printf("Not a palindrome\n");
}
LatinSuD
  • 1,779
  • 12
  • 19
0

No pointers (except the one use for making a copy of the original string).

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

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

    if ( argc != 2 )
        return ( 1 );   //  not properly invoked

    if ( (s2 = strdup( argv[1] )) == NULL )
        return ( 2 );   //  failed (not likely)

    printf( "\"%s\" %s a palindrome.\n", argv[1], strcmp( argv[1], strrev( s2 ) ) ? "is not" : "is" );

    free( s2 );

    return ( 0 );
}
BillP3rd
  • 1,009
  • 1
  • 9
  • 21