HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Counting letters, words, etc. in the input

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thecountingwordsinputlettersetc

Problem

I'm trying to learn some coding to broaden my scope of knowledge, and I seemed to have run into a bit of a conundrum.

I'm trying to create a program to output the number of characters, digits, punctuation, spaces, words and lines that are being read in from a file.

Here is the text file I am reading in:

See Jack run. Jack can run fast. Jack runs after the cat. The cat's fur is black. See Jack catch the cat.
Jack says, "I caught the cat."
The cat says, "Meow!"
Jack has caught 1 meowing cat. Jack wants 5 cats, but can't find any more.


Here is my code:

#include
#include

using namespace std;

int main()
{
ifstream lab3;
string word;
lab3.open("lab3.txt");
int countletters=0,countnum=0,countpunc=0,countspace=0,words=0,line=0;
char character,prevchar = 0;
if(!lab3)
{
cout

Output:

There are 167 letters.
There are 2 numbers.
There are 18 punctuations.
There are 52 spaces.
There are 47 words.
There are 4 sentences.


Some things I am hoping to learn:

  • Advice for improvements on my code for learning purposes/efficiency.



  • Explanation for reading information in from a text file: whether it is letters, numbers, punctuation - whatever you may run across doing this type of data-processing.



Some things I am aware of:

  • using namespace std;` is not good practice - what is the best practice for real world applications?



  • I am a beginner and this may not be (definitely is not) the cream-of-the-crop coding.

Solution

About using namespace std;. It's best to just do, e.g., std::ifstream etc each time. There's a lot of stuff here, and even in this example it would have made it easier for me to see that std::ispunc was built-in, and not something you wrote for this.

Variable names: If your name is two words put together, you should use either snake_case or camelCase consistently. E.g. countletters -> countLetters. I'd find numLetters even more readable. Some people would even say you really ought to make it numberOfLetters. lab3 also ought to be more descriptive.

Arrange your code logically. First you declare lab3. Then you declare word. Then you declare a lot of other stuff. And finally, you make sure lab3 opened correctly. Doing all the ifstream stuff in one place makes it so much easier to read/fix later on. E.g.

std::ifstream input("lab3.txt");
if (!input)
{
    std::cerr << "Could not open file." << std::endl;
    return 1;
}


For reading the file, personally I would use std::getline, then split each line into a vector of "words." First of all, this would it easier to check whether to count something as a word (I think yours breaks on contractions.) And then you can just loop over each character of each word of each line for the small stuff.

Misc. std::ifstream's close() gets called by the destructor when it goes out of scope. No need to do it manually here. Your output says you have 4 sentences. You have 4 lines. Several more sentences. There's probably not much to do about it here, but whenever I see a whole bunch of variable definitions grouped together, my first thought is to move them closer to where they're actually used. But again they're already about as close as they can get here. You never use word.

Code Snippets

std::ifstream input("lab3.txt");
if (!input)
{
    std::cerr << "Could not open file." << std::endl;
    return 1;
}

Context

StackExchange Code Review Q#153530, answer score: 5

Revisions (0)

No revisions yet.