Reading .pgn files to display statistics about Chess games

I am currently writing my first actual C++ project. I like Chess and I would like to see how good I'm doing on a monthly basis, so the idea is to read the log files (.pgn files) from the games and tell me how many games I won and such.

My question specially relates to Games.cpp where I'm not sure I have done the right thing in having both a getMonths() returning a list of all the months I have played a game and a getGamesByMonth(std::string month) returning all the games played on a given month. Seems like too much work - maybe a simpler solution is available.

Also, looking at wonAsBlack(), wonAsWhite(), lostAsWhite(), etc, seems like too much work as well - there might be a better solution to this as well. You can see the actual way it works in main.cpp where I have made a test case.

The code is here.

Games.cpp:

```
#include "Games.hpp"
#include "Parser.hpp"

Games::Games() {

}

Games::Games(std::vector games) {
pgames = games;
}

void
Games::openFile(std::string tfile) {
Parser parser;
parser.parse(tfile);
pgames = parser.games();
}

size_t
Games::getSize() {
return pgames.size();
}

Games
Games::getGamesWonBy(std::string winner) {
std::vector games;
for(Game &game : pgames) {
if(game.getWinner().find(winner) == 0) {
games.push_back(game);
}
}
return games;
}

Games
Games::getGamesLostBy(std::string loser) {
std::vector games;
for(auto &game : pgames) {
if(game.getLoser().find(loser) == 0) {
games.push_back(game);
}
}
return games;
}

std::vector
Games::getMonths() {
std::vector months;
for(Game &game : pgames) {
std::string month = game.getMonth();
auto index = std::find(months.begin(), months.end(), month);
if(index == months.end()) {
months.push_back(month);
}
}
return months;
}

Games
Games::getGamesByMonth(std::string month) {
std::vector games;
for(auto &game : pgames)

Games.cpp

-
It's not necessary to have member functions access private data members by their getters. These particular functions already have full access to the class.

-
Too many of your member functions begin with get, yet they don't all perform simple member-accessing. This is a sign that your functions need to be renamed to accurately identify their purpose. Consider this function of yours:

if (game.getDraw() == true) {}

Unless draw is a bool, this could be misleading. Make it clear that it's supposed to perform a Boolean operation.

-
if (game.getDraw() == true) {} can simply be if (game.getDraw()) {}.

-
I'd recommend making std::vector a typedef (under private).

-
This:

Games::Games(std::vector games) {
    pgames = games;
}

should be an initializer list instead:

Games::Games(std::vector games) : pgames(games) {}

-
getSize() should instead be defined in the class header (also as const):

std::size_t Games::getSize() const {return pgames.size();}

-
When a non-native type (such as std::string) parameter will not be modified, have it passed by const-reference:

Games::getGamesWonBy(std::string const& winner) {}

-
If these are supposed to be conditionals (kinda hard to tell for sure):

if (game.getWinner().find(winner) == 0) {}
if (game.getLoser().find(loser) == 0) {}

make it clear that they are (false in this case):

if (!game.getWinner().find(winner)) {}
if (!game.getLoser().find(loser)) {}

Main.cpp

-
It'd be a good idea to alphabetize your #include s for organization.

-
Unless you're okay with flushing the stream with each endl, use \n in the couts instead.

-
I'd suggest grouping these lines appropriately based on their roles for readability (especially the outputs). This is important because you need to make sure your driver code is written as intended, as this is what tests your class.

-
It appears that line isn't used anywhere. If it's a leftover, then get rid of it. This is a reason why variables should always declared near the line(s) where they are used.

-
The very last output seems out of place when preceded by that huge block of outputs. You could instead test for a failed condition first. If it does fail, display that message and return EXIT_FAILURE (or 1). If it doesn't fail, run the program and then terminate with the return 0.

StackExchange Code Review Q#29290, answer score: 4