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][1] to get into. * Overusing `std::endl` can likewise be a [bad habit][2] 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 use `std::endl` when you specifically want a flush to happen (for example, your print function could use all `\n` and then have one `std::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][3] ------ As an aside, it might be worth noting that there's actually a super useful pattern of reading things from a stream. It's relatively poor performance considered to something specialized like `std::strtol`, but it can be quite handy if you're in a bind (i.e. can't use Boost or some other library) and need something highly generic: template<typename T> bool readLine(std::istream& is, T& val) { std::string line; if (!std::getline(is, line)) { // throw exception ... } std::istringstream ss(line); return (ss >> val); } It also might be worth noting that instead of a bool, functions like this will often return the input stream with certain bits set. In other words, some people prefer to set the fail bit instead of return false. [1]: http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice [2]: http://stackoverflow.com/questions/213907/c-stdendl-vs-n [3]: http://stackoverflow.com/questions/1107705/systempause-why-is-it-wrong