strchr not working in C

Question

So right now I'm trying to code a program in C that takes a string and checks for proper punctuation (e.g. ends with '.', '?', or '!'). I am trying to use the strchr function to test and see if the last character in the string is one of the punctuation marks using a if loop within a for loop. However when I run the program it seems to skip the if loop all together.

Here is the program:

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

int main(void)
{
 char string[1000];
 int i,length,a,p,q,e;

 printf("Please enter a sentence with a valid punctuation.\n\n");


 for(i=0;i<1000;i++)
 {
  fgets(string,2,stdin);
  p=strchr(string,'.');
  q=strchr(string,'?');
  e=strchr(string,'!');
  if(string[sizeof(string)-1]=='.'||'?'||'!')
  {
   printf("\nYay a sentence!");
   break;
  }
  else if((p && q && e)==NULL)
  { 
   printf("You didn't provide any punctuation. Goodbye.");
   exit(a);
  }
 }
 printf("You entered the sentence:\n %s",string);

 return 0;
}

I have tried it so many different ways such as trying strstr instead or even storing it a different way through gets (which I quickly learned through gcc and some research is not the way to go.)

I am just completely lost on why this isn't working when I enter a sentence without punctuation.

Sorry if this is really simple, I am fairly new to this.

Thanks in advance.

Answer 1

You misunderstood the return value of strchr: it does not return an index of the character; instead, it returns a pointer to the character that you search.

char *p=strchr(string, '.');
char *q=strchr(string, '?');
char *e=strchr(string, '!');

In addition, sizeof does not return the actual length of the string; it returns 1000, which is the size of the string array. You need to use strlen instead.

Finally, string[strlen(string)-1]=='.'||'?'||'!' does not compare the last character to one of three characters. It always returns 1, because character codes of ? and ! are not zero, and so the logical OR || operator treats them as true value.

Same goes for (p && q && e)==NULL) condition: it does not check that all three values are NULL; one of them being NULL would be sufficient to produce an equality, but it's not what you want.

Here is how you fix this:

char last = string[strlen(string)-1];
// Skip '\n's at the end
while (last != 0 && (string[last] == '\n' || string[last] == '\r')) {
    last--;
}
if (last == '.' || last == '?' || last == '!') {
    ...
}
// Using implicit comparison to NULL is idiomatic in C
if (!p && !q && !e) {
    ...
}