0

I'm trying to copy unique words from a .txt file to a linked list. I need to copy every single unique word once without any repetitions. I wrote a program, but I have a problem with the function which checks if the word already exists in the list. Below is the code snippet which reads words from the file and adds the word to the linked list if it's not already added.

FILE*fd;
fd=fopen("c:\\texto.txt","r");
while(!feof(fd))
{
    fscanf(fd,"%s",aux);
    if(check(&lista,aux)==1){
    add(&lista,aux);}
}

My idea of the check function is: check if the actual word I got from the file is the same as the one I already have in the current node in the linked list. If it is not present in the current node, go to the next node. If the function finds that the word is already in the list, return 0. In case the function reaches the end of the linked list, return 1, so that the main can add the word to the linked list.

   int check (t_nodo*lista, char*aux)
{

    if((*lista)!=NULL&&strcmp(((*lista)->word,aux))!=0)
    {
        check(&(*lista)->next,aux);
    }

    if((((*lista)->word,aux))==0)
    {
        return 0;
    }

    if((*lista)==NULL)
    {
        return 1;
    }

}//el tano se la come a mordiscones

Actually, the program is copying only the first word to the list. Then it sets the word of all the other nodes same as the first word. I've already checked the "add" function and it's ok.

phoenix
  • 3,069
  • 3
  • 22
  • 29
eltanoselacome
  • 1
  • 3
    `if((((*lista)->word,aux))==0)` should be `if(strcmp((*lista)->word,aux)==0)` – phoenix Feb 04 '16 at 18:52
  • 1
    Is your `t_nodo` typedef a pointer type? If so, then I don't see why your argument to `check()` is a `t_nodo *`, but if not then you are using `*lista` where you mean just `lista`. – John Bollinger Feb 04 '16 at 18:53
  • Please see [Why is “while ( !feof (file) )” always wrong?](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). And please check the return values from `fopen` and `fscanf` etc **every time** – Weather Vane Feb 04 '16 at 18:53
  • 1
    also please add your `add` function. Without that how can we give inputs – phoenix Feb 04 '16 at 18:53
  • 1
    Please post the [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve) that demonstrates the fault. You don't know where it is, so you can't judge which functions are responsible. You have not shown how `lista` is defined, or what a `t_nodo` is, so it is impossible to tell if `(*lista == NULL)` should be `(lista == NULL)` and so on. – Weather Vane Feb 04 '16 at 19:01
  • regarding these line: `fd=fopen("c:\\texto.txt","r"); while(!feof(fd)) { fscanf(fd,"%s",aux);` 1) always check (!=NULL) the returned value from `fopen()` to assure the operation was successful. 2) Do NOT use `feof()` as a loop control, it does not do what the code is expecting. Suggest: `if( NULL == (fd=fopen("c:\\texto.txt","r") ) ) { // handle error, cleanup, and exit } while( 1 == fscanf(fd,"%s",aux);` However, '%s' will allow overflow of the aux[] buffer resulting in undefined behaviour. Suggest: use length modifier on %s of sizeof(aux)-1. -1 because '%s' will append a NUL byte – user3629249 Feb 04 '16 at 19:45
  • this line `if((((*lista)->word,aux))==0)` does nothing, suggest removing that `if` code block. this line: `if((*lista)==NULL)` was already checked in the first `if`, suggest remove that code block. This recursive function fails to pass back the result of the call to `check()`, so the recursion logic is not correct – user3629249 Feb 04 '16 at 19:52

1 Answers1

3

You go deeper in the list but not returning the value. Fix:

if((*lista)==NULL)
{
    return 1;
}

if(strcmp(((*lista)->word,aux))!=0)
{
    return check(&(*lista)->next,aux);
}

return 0;

Also consider using while loop instead of recursion.

Zbynek Vyskovsky - kvr000
  • 18,186
  • 3
  • 35
  • 43