Checking for an existing word in a file

Question

This piece of code basically just asks for a word and sees if it exists in a file. If not, the user is asked if it should be added to the file.

I am new to C programming and I want to know what you think about my code: things to change, things to remove, what not to do.

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
int main(void)
{
 //Declarations
 //**********************************************************
 char wordContainer[20],wordFinder[20],ans[2];
 int wordChecker=0;
 FILE* filePtr=NULL;
 //**********************************************************
 filePtr=fopen("wordDic.dic","a+");//opening the file with the a+ option
 printf("please enter the word you want to look for : ");
 fgets(wordContainer,20,stdin);//entering the word we want to look for
 if(filePtr==NULL)
 {
 printf("error while trying to open the dictionnary file");//allways good to check
 }
 /*while we haven't reach the end of the file see if the current word matchs the word we are looking for*/
 while(fgets(wordFinder,20,filePtr)!=NULL)
 {
 if(strcmp(wordContainer,wordFinder)==0)
 {
 printf("this word was found in the dictionnary , would you like to perform another search ? y/n... ");
 fgets(ans,2,stdin)
 wordChecker++;//when the word is matched increase wordChecker and quit the while
 break;
 }
 }
 if(wordChecker==0)//if no match is found ask to enter this new word
 {
 printf("we couldn't find the word %s would you like to add it ? y/n ... ",wordContainer);
 fgets(ans,2,stdin);
 if(strcmp(ans,"y")==0 || strcmp(ans,"Y")==0)
 {
 fprintf(filePtr,"%s",wordContainer);
 fflush(filePtr);//not exactly sure what it does exept that it clear the output buffer maybe ?
 printf("this word has been added");
 }
 else
 {
 printf("thank you for using this program,bye");
 exit(0);
 }
 }
 fclose(filePtr);
 return 0;
}

Answer 1

Your code looks quite neat, although note that formatting, including spaces around operators etc, line lengths, defining multiple variable per line, should follow a conventional style.

There are some issues, the main one being that compiling and running the code, it does not work. The problem is that

fopen(filename, "a+");

opens the file for reading and writing but leaves the stream pointer at the end of the file. So any subsequent read will fail. You need a rewind(); or equivalent (fseek etc) after checking that the file opened successfully.

After fixing the rewind, another issue is that fgets returns the input word with a trailing newline, \n. Your code ignores this and manages to work because the strings that are written to and read from the dictionary also have a trailing \n. But this fails when a user enters a string that fills your buffers, 19 characters or more. In this case, the fgets call cannot read the whole string and reads just the 1st 19 chars (and adds a terminating 0円), leaving the remaining chars in the input stream. In this case there is no trailing \n in the buffer. The next time you read from stdin the 20th and 21st chars are read and treated as the y/n answer. Even if these chars happen to be "y", the 19-char word is written to the dictionary without a trailing \n, corrupting the dictionary (try looking up abcdefghijklmnopqrsy - notice the 'y' at the end)

There are ways to fix these problems, for example you could try using getline() to get the input string instead of fgets, although getline is more tricky to use correctly. Alternatively (as it is just an exercise) just check the input string to confirm that there is a trailing \n (meaning that the whole line was read) and reject the word and fflush the input if not. To do things properly you really need to remove the trailing \n from the user input string and from the string read from the dictionary before comparing, and add \n back when writing to the file.

Another issue is poor comments. Comments must be useful; otherwise they are just noise (and hence make reading the code more difficult than it need be). All of your comments are just noise.

Note also that when opening the dictionary fails you should exit. There are numerous other issues, but I'll leave it at that for now...

EDIT

As no reviewers have commented on other issues, here are some other things I spotted:

• avoid use of constants such as 20. Instead, define the buffer size with a define and use sizeof where you need the buffer size. For example:

  #define BUF_SIZE 20
  ...
  char buf[BUF_SIZE];
  fgets(buf, sizeof buf, stdin);
  

  by using sizeof you guarantee that the size is correct - but note that if you use it on a pointer, then you get the wrong value:

  char *buf = malloc(BUF_SIZE);
  fgets(buf, sizeof buf, stdin); // WRONG!!
  

  in the latter case, sizeof returns the size of the pointer (typically 4 or 8 bytes) - you need to pass BUF_SIZE to fgets

• define the file name in a constant or a #define and use the file name in the error message on failure to open. The message should be produced with perror

  const char *dict = "wordDic.dic";
  ...
  FILE *file = fopen(dict,"a+");
  if (file == NULL) {
   perror(dict);
   exit(EXIT_FAILURE);
  }
  

  notice that I exited immediately on failure (your code continues). perror will produce a short error message on stderr that says why the file could not be opened, not just that it couldn't. Also note that I checked the file pointer immediately, not after prompting the user, as you do.

• you ask the user whether she would like to perform another search (on success) but then ignore the answer.

• the call to fflush is strictly unnecessary. Buffered output like fprintf (and printf) writes to file when it sees a \n in the buffer. In this case, your strings have \n embedded (from the fgets call) and so fprintf will flush the buffer; the buffer is empty when fflush is called. The normal thing would be for the word in wordContainer to have no \n and for fprintf to use a format containing \n - fprintf(filePtr, "%s\n", word); - and again, no fflush is needed.

• your

  else
  {
   printf("thank you for using this program,bye");
   exit(0);
  }
  

  exits before the final closing of the file and zero return. Closing the file is good practice, although it will happen when the program exits anyway.

• I don't really like your variable names - three variables starting with 'word' is confusing. I'd go for something like word for the user-entered word, entry for the dictionary entry read from file and found to indicate that the word was found in the dictionary. That is a rather personal preference though...

• remember to put a \n on the end of lines that are printed, eg on exiting the program.

• apprentice: thanks a lot for you very helpful answer ,about the rewind problem , it was there , realy ! But i think it was lost during the parsing as for the \n problem I'll probably write a function that goes trough the entered word and the words retrieved from the file and replaces the \n with a 0円, and thanks again for your answer
• apprentice: William Morris, I can never thank you enough for your edit , it's exactly the kind of answer I'd hoped to get , I'll try to apply everything you said on my future programs :D