Return to Answer

###General Style My main problem with the code as it stands is that it is messy (and thus hard to read). The contributing factors are:

###Comments on code

General Style

My main problem with the code as it stands is that it is messy (and thus hard to read). The contributing factors are:

Comments on code

Declare variables as close to the point of use as you can. Don't be scared of return by value. The optimiser is going to illiminate the copy out of the function for you.

Have to go. More latter.

Declare variables as close to the point of use as you can. Don't be scared of return by value. The optimizer is going to eliminate the copy out of the function for you (look up RVO NRVO). Also out parameters are a pain when maintaining the code. Its easy to spot when writing but noticing out values at the call point several months later is a pain (much easier to use return values).

OK you are reading a file. But that sorts of limits reuse. Why not read from a stream (then you can pass std::cin as the stream but also any other type of stream.

void ReadFile(std::vector<string> &lines_in_file) {

Copy the whole stream to another stream. Seems like a bit of a waste. Then read a line at a time. Why not just read a line at a time from the original stream std::cin?

std::vector<std::string> ReadFile(std::istream& input) {
{
 std::vector<std::string> result;
 std::string text_from_input;
 while (std::getline(input, text_from_input, '\n')) {
 lines_in_file.push_back(text_from_input);
 }
 return result;
}

But now looking at that we see, a loop over a stream. There must be an algorithm for that. We could just replace the while() with std::copy() (I'll define Line in a second).

std::vector<std::string> ReadFile(std::istream& input) {
{
 std::vector<std::string> result;
 std::copy(std::istream_iterator<Line>(input), // begin
 std::istream_iterator<Line>(), // end
 std::back_inserter(result)); // 
 return result;
}

But we can take this a step further as the vector can take two iterators as part of its constructor.

std::vector<std::string> ReadFile(std::istream& input) {
{
 std::vector<std::string> result(std::istream_iterator<Line>(input), // begin
 std::istream_iterator<Line>()); // end
 return result;
}

Now we just need a definition for a Line. This is basically an object that is convertable to std::string and when used with operator>> reads a line of text.

struct Line
{
 std::string line;
 operator std::string const& {return line;}
 friend std::istream& operator>>(std::istream& s, Line& data)
 {
 return std::getline(s, data.line);
 }
};

Badly named function.

bool ValidateArguments(int arguments) {

Does not seem to validate the arguments. Also it prints out help. I would have two different functions for this. Especially since you may want to print out the help under other situations.

Looks like the first arguments to this function is not modified. So why is it passed by reference. Best to pass parameters by const reference to avoid the copy and prevent accidental modification. Prefer not to use out parameters return the result.

void SplitInputLine(std::string &user_specified_pattern, std::vector<string> &users_pattern_split) {
 char sep = ' ';

This is OK but unusual: user_specified_pattern.npos. I would normally expect std::string::npos.

That is a most complicated expression you are trying to evaluate in one line.

users_pattern_split.push_back(user_specified_pattern.substr(p+(p != 0),
 (q = user_specified_pattern.find(sep, p+1))-p-(p != 0)));

That is just horrible to read split it up into multiple lines. SO it is readable.

std::size_t start = p + (p != 0);
q = user_specified_pattern.find(sep, p+1)
std::size_t end = q - p - (p != 0);
std::string sp = user_specified_pattern.substr(start, end);
users_pattern_split.emplace_back(std::move(sp));

Don't worry. With RVO/NRVO and move semantics. This will become just as efficient as the original. But now is much more readable. Which is nice as we can now spot mistakes easier.

Seems like we are special casing p in two sub expressions but not the third. Which is odd looking. Why not modify the meaning of p (it always point to the start of the next section (or end).