Printing certain number of words from file

Other code review noted problems with commenting and non-C++ code. This post aims to follow up on those issues.

Please check:

• Comments are appropriate

• Code is not C

My test file contains:


one two three four five six seven eight nine

```
//This app is an exercise/study: Etude fr1_x
// prints x number of words from file, without newline, as determined by
// short wordCount
// in:
// void printWord(std::vector &word)
// the output is connected to a timer.
#include
#include
#include
#include
#include
#include

//number of times to repeat output when calling println();
//instead of writing:
//
// println();
// println();
//
//i can write:
//
// println((count)2);
//
//
//also, because println(int i) is already defined
typedef unsigned short count;

//TODO - yank all print[ln] to header
void print(std::string str){
std::cout maxVal){
println((count)2);//accidental recursion
println("Error:");
println("called 'println(count c) with invalid arg");
println("required: param c> input;
return input;
}

std::string getFileName(){
return prompt("\nEnter a file name");
}

//TODO - break it up - too nested - too conditional
void readFile(std::vector &word){
const std::string quit("quit");
std::string fileName;
std::string str;
short kontinue = 1;
do{
fileName = getFileName();
kontinue = fileName.compare(quit);
if(kontinue != 0){
std::ifstream file(fileName.c_str(),std::ios::in);
if(!file.is_open()){
continue;
}
//source:Thinking in C++, 2nd ed. Volue 1, Ch. 2: Introducing vector
while(file >> str){
word.push_back(str);
}
file.close();
break;
};
}while(kontinue != 0);
return;
}

void test(std::vector &word){
int size = word.size();//could be a problem, maybe use long
println();
for(int i=0;i &word){
//how ma

Comments:

-
The top comment block is good. When writing a program, describing it at the top will tell the reader what it is expected to do output.

-
Too much whitespace within some blocks. Some of your comment text are too spread-out, adding more lines and reducing readability. Avoid this by making your comments as concise as possible.

-
Talking to yourself "aloud". Comments written in the first-person could be rewritten more professionally:

// short-term control - assign negative value to count for testing

-
"TODOs": They are okay, but should be kept simple:

// TODO - break up nested and conditional parts

-
"val will become user input eventually" and book references: We don't need to know these things. They only help you, not us.

Code is not C:

Compared to your previous program revision, this does look more C++-like. @Lstor's main point was to use std::string, which you're now doing in place of char arrays.

I'd recommend getting more familiar with C++'s STL. std::string is part of this library, and there's much more to explore. Use it whenever possible, and your code will become more C++-like, more concise, and more robust. Here are two starting points:

• Use this reference guide which describes each library and its purposes

• Pick up Effective STL to learn how to use it properly

Miscellaneous:

-
Your #includes could be sorted alphabetically for organization.

-
Be aware that ` is platform-specific. This is not recommended if your program needs to attain portability.

-
By default, void functions return on their own. It's only necessary when the function may need to end early for whatever reason.

-
The "print" functions are unnecessary and just clutter the program. Just use std::cout.

-
In println()'s if-block: It's best to avoid terminating with exit(). Replace it with a return and try to fall back to main() for termination.

-
printWord()'s reference parameter should be const. This applies to any non-native data type (such as an object) that will not be modified within the function.

-
std::getline() is preferred for a user-inputted std::string:

std::string input;
std::getline(std::cin, input);

-
size() returns an std::size_type, not an int. Correct return types should always be used, and you especially shouldn't mix signed/unsigned types.

std::vector::size_type size = word.size();

You also don't need to create a variable for this. Just use the word.size() in the for-loop.

-
readFile() is hard to follow and could be simplified:

std::vector readFile(std::istream& inFile)
{
    std::vector word;
    std::string fileStr;

    while (inFile >> fileStr)
    {
        word.push_back(fileStr);
    }

    return word;
}

It's best to check the file in main() so that it can terminate if the file isn't found. In this function, an input stream (for the file created in main() is received and an std::vector` is returned. This is okay to do because of Return Value Optimization.

StackExchange Code Review Q#28900, answer score: 11