C++ count chars, words, sentences in file using C-strings

Question

I am to use "C-strings, not C++ string objects" per teacher to read a paragraph from an input file and count characters, words, sentences, and the number of to-be verbs (to, be, am, are, is, was, were). My current program clears compiler but my character count and words count both off by two. My code to count sentences stops after counting one sentence. Any help as to my errors is appreciated. Still working to debug. My teacher basically told the class what strtok did and then literally told us to figure it out from there. Not asking for you to do it for me -- just tips/hints to get back in the right direction. Happy Easter.

#include <iostream>
#include <cstring>
#include <fstream>
#include <cstdlib>


using namespace std;

const int MAX_FILENAME = 256;
//Longest word length = 15 chars
const int MAX_WORD_CHARS = 16;
//Longest word in paragraph = 200 words
const int WORDS_MAX = 200;
//Max amount of chars in paragraph = 15*200
const int MAX_PARAGRAPH_CHARS = 3000;
//Max chars in a sentence
const int MAX_SENTENCE_CHARS = 200;
const int MAX_SENTENCES = 25;
const int NUM_TO_BE_VERBS = 5;

void readParagraph( ifstream& input, char [] );
int countWords( char [], char tp[][MAX_WORD_CHARS] );
int countSentences( char [] );

int main()
{
   int i;
   int words, sentences, average;
   char filename[MAX_FILENAME];
   ifstream input;
   //Holds paragraph characters 
   char p[MAX_PARAGRAPH_CHARS]; 
   const char TO_BE_VERBS[NUM_TO_BE_VERBS][MAX_WORD_CHARS] = { "am", "are", "is", "was", "were" };
   const char BE[] = "be";
   const char TO[] = "to";
   char tp[WORDS_MAX][MAX_WORD_CHARS];

   //Prompt user input file name
   cout << "Enter input file name: ";
   cin.get( filename, 256 );


   //Open input file
   input.open( filename );
   //Check input file exists
   if ( input.fail() )
   {
      cout << "Input file " << filename << " does not exist." << endl;
      exit(1);
   }

   //Reads paragraph into array  
   readParagraph( input, p );

   countWords( p, tp );


   countSentences( p );

   return(0);
}


void readParagraph( ifstream& input, char p[] )
{
   int count = 0;

   while ( input.get( p[count]) && (count < MAX_PARAGRAPH_CHARS) )
   {
      count++;
   }
   p[count - 1] = '\0';

   cout << "Number of characters: " << count << endl;
}

int countWords( char p[], char tp[][MAX_WORD_CHARS] )
{
   int i = 0;
   char* cPtr;

   cPtr = strtok( p, " " );
   while ( cPtr != NULL )
   {
      strcpy( tp[i], cPtr );
      i++;
      cPtr = strtok( NULL, " " );
   }

   cout << "Number of Words: " << i << endl;   
   return(i);
} 

int countSentences( char p[] )
{ 
   int j = 0;
   char* Ptr;
   char sent[25];

   Ptr = strtok( p, ".!?" );
   while ( Ptr != NULL )
   {
      strcpy( sent, Ptr );
      j++;
      Ptr = strtok( NULL, ".!?" );
   }

   cout << "Number of sentences: " << j << endl;

   return(j);
}      

Answer 1

There are a number of problems with the code and a problem with the assignment. First, as has been mentioned, this is not the way that a professional programmer would approach this task, so shame on your instructor for not teaching students real-world use of C++. That said, there is still plenty to be learned with such an assignment. Here are some issues for you to consider and correct.

Unused variables

Unused variables are very often a sign of poor quality code. They clutter up the code, making it hard to understand and add nothing of value. Your compiler can help you find these kinds of problems if you ask it to be particularly picky. I used g++ to compile your code and used the following command line:

g++ -Wall -Wextra -pedantic --std=c++11 -o stringcount stringcount.cpp

It helpfully pointed out that i, words, sentences, average, TO_BE_VERBS, BE and TO are unused.

using namespace std;

Incorporating using namespace std; into every program you write is a bad habit that you should avoid. If your instructor is advising that you use this construct, be aware that you're going to have to unlearn this kind of bad habit if you ever decide to write code other than for that class.

Reading paragraphs

Your readParagraph routine includes this loop:

while ( input.get( p[count]) && (count < MAX_PARAGRAPH_CHARS) ) 

but consider that this will only stop if it reaches the end of the file or exceeds the MAX_PARAGRAPH_CHARS which is not the usual definition of a paragraph. Instead, you probably want to use some other indicator, such as a blank line, to indicate a paragraph has ended.

Separation of output from core logic

Many of your routines print as well as doing some kind of calculation. While this is a reasonable way to help troubleshoot non-working code, generally it's better to separate the calculation (as with readParagraph) from printing. That means that what would likely make more sense is to have readParagraph return an unsigned that represents the number of characters in the paragraph and not print anything. If something needs to be printed, print it from some other routine or from main.

Error handling

Look at your readParagraph function:

void readParagraph( std::ifstream& input, char p[] )
{
   int count = 0;

   while ( input.get( p[count]) && (count < MAX_PARAGRAPH_CHARS) )
   {
      count++;
   }
   p[count - 1] = '\0';

   std::cout << "Number of characters: " << count << std::endl;
}

Is that \0 character really going in the right place? Where does it get written if there are no characters read?

Think carefully about the problem

Can you count the words in this sentence? I count eight. Your program might not because your countWords function includes this line:

cPtr = strtok( p, " " );

Note that words might end with a period or question mark and not necessarily with a space character. The strtok function can actually look for multiple delmiters within the same call, so you may wish to adjust that call.

Creating extra work

The countWords function carefully copies each discovered word into the tp array, but the tp array is then never used. If you don't use it, don't create it. It will make your program more durable and easier to troubleshoot as well as being slightly faster. The same is true of the mysterious sent variable in your countSentences function.

return is not a function

Don't write return(j); because return is not a function. Instead, just write return j; which is more idiomatic C and C++ style.

Magic numbers

You've used a number of const int declarations for constants in your program, which is a good thing. However, there are still some mysterious and unexplained "magic numbers" that are embedded in the program such as the 256 in this line:

std::cin.get( filename, 256 );

(Note that I've added std:: in front to show what it would look like when you omit the using namespace std; from your program.)

Answer 2

Here's how to debug code like this (which is related to, but not the same as, the way to write code well and avoid this predicament in the first place).

Step 1: pick an observed problem and lock it down.

The character count is off. Given the text "red apple", the code reports Number of characters: 10

Step 2: Find the relevant code and strip it down as far as you can, while still displaying the behavior.

void readParagraph( ifstream& input, char p[] )
{
   int count = 0;

//   while ( input.get( p[count]) && (count < MAX_PARAGRAPH_CHARS) )
   char c;
   while ( input.get(c))
   {
      count++;
   }
//   p[count - 1] = '\0';

   cout << "Number of characters: " << count << endl;
}

Step 3: put in some diagnostic output statements so as to see what's going on (or, for more advanced students, run the whole thing in a debugger).

void readParagraph( ifstream& input, char p[] )
{
  int count = 0;

  char c;
  while(input.get(c))
  {
    count++;
    cout << count << ": " << c << endl;
  }

  cout << "Number of characters: " << count << endl;
}

The output is:

1: r
2: e
3: d
4:  
5: a
6: p
7: p
8: l
9: e
10: 

The code is counting spaces and carriage returns as characters.

Step 4: fix it.

How you deal with this depends on what you want it to count.

Go back to Step 1, repeat until you're satisfied.