Looks pretty good overall. The biggest issue that I see is that the input reading doesn't check for errors, and it's pretty rough. It can be helpful with assignments like this to build up a little library of IO helpers:
// Read a line from the provided stream.
std::string readLine(std::istream& is) {
std::string line;
if (!std::getline(is, line)) {
// Could stand to be a more specific exception, but you get the point
throw runtime_error("Unexpected stream failure");
}
return line;
}
// Helper to output a prompt and read a line.
std::string promptLine(std::ostream& os, const std::string& prompt, std::istream& is) {
os << prompt; // Note that I've assumed output won't fail. Depending on situation,
// that might be a bad assumption.
return readLine(is);
}
// Attempt to parse an int from a line of input that must contain only an int.
bool readIntLine(std::istream& is, int& val) {
std::string line = readLine();
char* parseEnd = nullptr; // NULL if < C++11
int val = std::strtol(line.c_str(), &parseEnd, 10);
return (parseEnd - line.c_str() == line.size());
}
// Keep showing the same prompt until the user inputs an integer (and only an integer)
int promptInt(std::ostream& os, const std::string& prompt, std::istream& is) {
int val;
do {
os << prompt;
} while (!readIntLine(is, val));
return val;
}
// If you wanted, you could take it a step farther and make convenience wrappers that assume std::cin/std::cout.
// Example:
int promptInt(const std::string& prompt) {
return promptInt(std::cout, prompt, std::cin);
}
This looks like quite a bit of code (and it is -- unfortuntely IO just kind of sucks to do non-carelessly), but it makes reading in the data a bit cleaner while also providing verification of data:
MovieData getMovieData()
{
MovieData movie;
movie.title = promptLine("Enter the title of the movie: ");
movie.directory = promptLine("Enter the name of the movie's director: ")
movie.year_released = promptLineInt("Enter the year the movie was released: ");
// ...
return movie;
}
Other minor issues:
- Using
using namespace std;
is a bad habit to get into. - Overusing
std::endl
can likewise be a bad habit as it not only writes a new line but also flushes the buffer. This can result in surprisingly bad performance when heavvy IO is involved. It's much better to use'\n'
by default and only usestd::endl
when you specifically want a flush to happen (for example, your print function could use all\n
and then have onestd::endl
at the end). - Some of your comments are a bit pointless and should be removed to reduce noise.
system("PAUSE")
is a bad habit