Count word frequency with linked list

[arbor33]

I'm supposed to code a program in C++ that queries a user for a file name. That file is then to be scanned and each word is to be placed into a node within a linked list. If the word is already in the list the counter of the respective node should increase by one and there should be no duplicate nodes.

I have coded the program to the best of my ability and got it to compile, however, it crashes when I run it... I've been scratching my head over it for hours and cant seem to get anywhere with it.

#include <iostream>
#include <stdio.h>
#include <stdlib.h>
using namespace std;

struct node
  {  char word[50];    // word being inputted
     int count;        // word frequency
     node *next;        // Pointer to next node
  };
node *start = NULL;

void add_node(char nWord[50]);

int main()
{ char cWord[50];
  char cFile[30];
  FILE *pfTextfile;  
  node *temp;     //temporary node to store data    
  node *current;       //node to traverse with
  int TravFlag = 0;  //Flag to help traverse

  cout << "Enter the file you wish to have searched:\n";
  cin >> cFile;
  
    // open file
  pfTextfile = fopen(cFile, "r");
  while(!feof(pfTextfile))
  { TravFlag=0;
    //empty cWord
   for (int i=0; i<50; i++)
		{ cWord[i]='\0';
		}
   current = start; //node that points to beginning 		
  // make temp first node
   if (start == NULL)
      {start = temp;
       start->word[50]=cWord[50];
       start->count=1;
      }
   else if(start!=NULL) //if theres already a node
         { while(TravFlag==0)//Loop to Traverse
           {if(current->word==cWord)//If current node is same word
              {current->count=current->count+1;//count increases by 1
               TravFlag=1;                   //set flag to end loop
              }
            else if(current->word!=cWord&&current->next==NULL)
              {//end of the list
               add_node(cWord);
               TravFlag=1;
              }
            else
               current = current->next; //move to next node
           }      
         }
  }
  //Print em
  current=start;
  do
  {if(current==NULL)
      cout<<"End of list."<<endl;
   else
    { cout<< "The word "<<current->word<<" appears "<<current->count<<" times."<<endl;
      current=current->next;
    }
  }  
  while(current!=NULL);
}

void add_node(char nWord[50])
{    node *temp, *temp2;   // Temporary pointers

     temp = new node;
     temp->word[50]=nWord[50];
     temp->count=1;
     temp->next = NULL;

     // Set up link to this node
     if (start == NULL)
         start = temp;
     else
       { temp2 = start;
         while (temp2->next != NULL)
           {  temp2 = temp2->next; // Move to next node
           }
         temp2->next = temp;
       }
}

I'm pretty new to this stuff so any help at all would be fantastic!
Thanks!

[Russco]

Do you have to make the list yourself. This is c++ not c and in c++ we have std::list<type> which suffices for almost all linked list needs.

Personally i think a list is a bad idea because the check for duplicates is very slow as you must start at the head of the list and compare each node in turn until you find duplicate or reach the end of the list. This is far better done with a tree structure which can be searched a lot quicker.

#include <stdio.h>
#include <stdlib.h>

are C headers and have no place in a c++ program. <cstdio> and <cstdlib> are the c++ versions.

[arbor33]

No, I dont need to make the list myself as long as it holds the word and the amount of times it appears. I guess if it also includes similar or appropriate functions I may even be better off...

Did you see any mistakes in my logic that would make the program crash though? I am likely to still use similar logic even when changing the list to the standard C++ version.

[GCDEF]

If you run it in the debugger it will show you exactly where it crashes and most likely why.

[arbor33]

Ah, I see... I've never done anything like that before. Thanks!

It says that I have a segmentation fault at the line of code that reads

       start->word[50]=cWord[50];

Does a segmentation fault mean that there is no value being assigned?



I also added this line in because I realized I never stored data into cWord.

	fscanf(pfTextfile, "%s", cWord);

[GCDEF]

For an array of size 50, valid indexes are between 0 and 49.

[Russco]

Never use feof() to test for end of file. Its a bug to do that. You should use the return value from your file reading to control the loop.

[arbor33]

Ok, I should have put a loop in to assign the different chars to the array which means I should have a word size count...

I tried this and still have the same error on the same line.

	//read the next word from file, store as cWord
	fscanf(pfTextfile, "%s", cWord);	

   current = start; //node that points to beginning 
   WordSize = 0;
   //get the size of the word
   while (cWord[WordSize]!='\0')
		{
			WordSize++;
		}		
  // make temp first node
   if (start == NULL)
      {start = temp;
       for(int wd=0;wd<WordSize;wd++)
         { start->word[wd]=cWord[wd];
         }
       start->count=1;
      }

And to get around the feof() issue, all I need to do is set a condition to terminate the while loop if I get a '\0' character into cWord?

[GCDEF]

Why not just use strlen?

[Russco]

To get around the feof issue doesn't take much....

do something like..

while ( fscanf( pfTextfile, "%s" , cWord ) != EOF )

as I said control the loop with your file read function and not feof.

As an aside, try to work out exactly why feof is a bug!

[arbor33]

Thanks, that saves me a few lines of code. I just need to get past the segmentation error now...

Could it be that a comma ',' as the last character in the array of chars is making the system throw this error?

[GCDEF]

Thanks, that saves me a few lines of code. I just need to get past the segmentation error now...

Could it be that a comma ',' as the last character in the array of chars is making the system throw this error?

I told you what the error is in post 6.

[arbor33]

Oh, sorry. I did change that to a loop that I mentioned above so that I could actually assign the complete value of cWord and not just the last char.

It now looks like this and still gives the same error.

   if (start == NULL)
      {start = temp;
       for(int wd=0;wd<strlen(cWord);wd++)
         { start->word[wd]=cWord[wd];
         }
       start->count=1;
      }

[GCDEF]

Oh, sorry. I did change that to a loop that I mentioned above so that I could actually assign the complete value of cWord and not just the last char.

It now looks like this and still gives the same error.

   if (start == NULL)
      {start = temp;
       for(int wd=0;wd<strlen(cWord);wd++)
         { start->word[wd]=cWord[wd];
         }
       start->count=1;
      }

You're doing too much work here. Just use strcpy to copy a null terminated string.

[arbor33]

Okay, I've changed it to this

       strcpy(start->word,cWord);

I'm still getting the segmentation problem but now it doesnt tell me where. What a headache this has become...