CODEWITHSUNDEEP
cstatic
Greg Brown
Greg Brown

Reputation: 1299

Checking for a string in an array of strings in C

I have a function that takes in a filename (string) and I want to have an array of filenames (strings) to check to see if the filename taht was passed in is in the array and if not add it to it. Here is what I got so far.

bool read_file(char * filename, bool warnings){
  //Check to see if filename is in array files
  static char * files[MAXFILES];
  static int counter = 0;
  for(int i=0;i<counter;i++){
    if(strcmp(filename,files[i]) == 0){
      fprintf(stdout, "Error\n");
      return 0;
    }
    else{
      files[counter]=filename;
      counter++;
    }
  }
  FILE * fp = fopen(filename,"r");
  if(fp == NULL){
    if(warnings){
      fprintf(stderr, "Can't open the file %s\n",filename);
      return 0;
    }
    else{
      return 0;
    }
  }
  else{
    fclose(fp);
    fp = NULL;
    return 1;
  }
}

For some reason it wont add filenames to files[] any help would be appricated.

Upvotes: 2

Views: 132

Answers (4)

Dave
Dave

Reputation: 11162

Your else is misplaced. This:

 for(int i=0;i<counter;i++){
    if(strcmp(filename,files[i]) == 0){
      fprintf(stdout, "Error\n");
      return 0;
   }
   else{
      files[counter]=filename;
      counter++;
   }
}

Will never be entered, and were it to be, it would always think that the file was in the list, because you append it before you finish checking. You would then find the one you appended, and think it was already there.

Instead, wait until you've finished checking the whole list until you append the filename:

for(int i=0;i<counter;i++){
   if(strcmp(filename,files[i]) == 0){
     fprintf(stdout, "Error\n");
     return 0;
   }
}
files[counter]=filename;
counter++;

The key is that the return 0 redirects the flow out of your function, rather than an else clause. You know that as long as the loop finishes and you're still in the function that the filename isn't there yet.

Upvotes: 1

Jonathan Leffler
Jonathan Leffler

Reputation: 753695

Look at the else clause of this loop:

  for(int i=0;i<counter;i++){
    if(strcmp(filename,files[i]) == 0){
      fprintf(stdout, "Error\n");
      return 0;
    }
    else{
      files[counter]=filename;
      counter++;
    }
  }

You want to add the file to the list only once you know it is not in the list, and you won't know that until you have gone through the entire list. So, that code should probably look like:

for (int i = 0; i < counter; i++)
{
    if (strcmp(filename, files[i]) == 0)
    {
      fprintf(stdout, "Duplicate file %s found at %d\n", filename, i);
      return 0;
    }
}
files[counter++] = filename;

Notice the better error message than just 'Error'. You will need to know what went wrong because this won't be the only place in the program where there could be errors. You can debate whether the number is helpful; the name most certainly is, but having the index number might help you too.

Upvotes: 1

vireshas
vireshas

Reputation: 816

change your for loop to 

 for (i=0;i<=counter;i++)

Upvotes: 0

Seth
Seth

Reputation: 46423

Take a look at the first time through your loop (when i == 0 and counter == 0):

static int counter = 0;
for(int i = 0; i < counter; i++) {
    // this never runs
}

i < counter is always false, because counter is incremented in the body of the for loop.

You'll need to rethink your logic. This function is doing a lot of stuff. Maybe something like this infront of the loop will help: 

if (counter == 0) {
    // first time in the function, just add the filename
    files[counter]=filename;
    counter++;
}
else {
    for (int i = 0; i < counter; i++) {
        ...
    }
}

However, you might have better luck splitting this into a couple functions. One to add a filename to the array, another to check if it's already there, a third to open the file, etc.

Upvotes: 1

Related Questions