C++ Quiz game w/questions in random order

Question

This is a 10 question quiz that assess's the user's knowledge of the C++ programming language. Upon the start of the program some ASCII welcome art prints.

The questions, answers, and correct answer are stored in a text file. The questions are loaded in random order. The user is prompted to answer each question, if the answer is right points are added and a win message is displayed. If the question is answered wrong, the user is told they are wrong and the right answer is displayed. If a user passes, it will print "You Passed" ASCII art stored in a text file, if the user fails, it will just tell them they failed.

There are 25 total questions stored inside the text file. I plan to add to add more to the .txt file, so that the quiz will have more variety and feel less repetitive. These questions mostly come from here and a textbook I have for programming logic (Programming Logic and Design 8th Edition).

Example of one of the questions in quiz_data.txt:

What command prints something to the screen?
cin
cout
char
print
b

quiz_passed.txt:

__ __ ____ ____
\ \/ /___ __ __ / __ \____ ______________ ____/ / /
 \ / __ \/ / / / / /_/ / __ `/ ___/ ___/ _ \/ __ / / 
 / / /_/ / /_/ / / ____/ /_/ (__ |__ ) __/ /_/ /_/ 
/_/\____/\__,_/ /_/ \__,_/____/____/\___/\__,_(_) 

One thing I would really like to do with this code is to separate the PrintResults() function from the InititializeQuizGame() function.

What I have right now works as it's intended, but I feel like PrintResults() should be separate from InitializeQuizGame() for better readability and logic.

Any thoughts on this or other criticism appreciated. This is my first major program and I would like to know how I can make this code more efficient or readable. Thanks in advance!

My Code:

#include <iostream>
#include <fstream>
#include <string>
#include <sstream>
#include <vector>
#include <algorithm>
#include <random>
namespace {
 const int s_questionScore = 10; // Points rewarded for each correct answer.
 const int s_failingGrade = 60;
 const int s_numQuestions = 10;
 const char* s_winMessage = "Correct!\n";
 const char* s_loseMessage = "Incorrect, the correct answer was ";
 const char* s_promptAnswer = "What is your answer?\n";
}
class Question {
public:
 int askQuestion(int num = -1);
 friend std::istream& operator >> (std::istream& is, Question& ques);
private:
 std::string question_text;
 std::string answer_1;
 std::string answer_2;
 std::string answer_3;
 std::string answer_4;
 char correct_answer;
};
void PrintArt(std::ifstream myfile);
void InititializeQuizGame(std::ifstream data);
void load(std::istream& is, std::vector<Question>& questions);
void Shuffle(std::vector<Question>& questions);
void PrintResults(std::vector<Question>& questions);
void clearScreen();
void PositionCursor();
int main()
{
 PrintArt(std::ifstream("welcome.txt"));
 InititializeQuizGame(std::ifstream("quiz_data.txt")); //Load questions from .txt file
 return 0;
}
void PrintArt(std::ifstream myfile)
{
 std::string line;
 if (myfile.is_open())
 {
 while (getline(myfile, line))
 {
 std::cout << line << '\n';
 }
 myfile.close();
 std::cin.get();
 clearScreen();
 }
 else
 {
 std::cout << "Error: File not found!\n";
 }
}
void InititializeQuizGame(std::ifstream data)
{
 if (data.is_open())
 {
 std::vector<Question> questions;
 load(data, questions);
 Shuffle(questions);
 PrintResults(questions);
 }
 else
 {
 std::cout << "Error: File not found!\n";
 }
 std::cin.get();
}
std::istream& operator >> (std::istream& is, Question& ques)
{
 std::string line;
 while (std::getline(is, line))
 {
 if (line.size() == 0)
 continue;
 break;
 }
 ques.question_text = line;
 getline(is, ques.answer_1);
 getline(is, ques.answer_2);
 getline(is, ques.answer_3);
 getline(is, ques.answer_4);
 is >> ques.correct_answer;
 return is;
}
void load(std::istream& is, std::vector<Question>& questions)
{
 Question q;
 while (is >> q)
 questions.push_back(q);
}
int Question::askQuestion(int num) //Ask the question. 
{
 int score = 0;
 std::cout << "\n";
 if (num > 0)
 std::cout << num << ".) ";
 std::cout << question_text << "\n";
 std::cout << "a. " << answer_1 << "\n";
 std::cout << "b. " << answer_2 << "\n";
 std::cout << "c. " << answer_3 << "\n";
 std::cout << "d. " << answer_4 << "\n";
 //Ask user for their answer.
 char guess = ' ';
 PositionCursor();
 std::cout << s_promptAnswer;
 std::cin >> guess;
 if (guess == correct_answer) {
 std::cout << s_winMessage;
 score = s_questionScore;
 std::cin.get();
 std::cin.get();
 }
 else
 {
 std::cout << s_loseMessage << correct_answer << ".\n";
 std::cin.get();
 std::cin.get();
 }
 return score;
}
void Shuffle(std::vector<Question>& questions) //Shuffle the questions. 
{
 std::random_device rd;
 std::mt19937 randomGenerator(rd());
 std::shuffle(questions.begin(), questions.end(), randomGenerator);
}
void PrintResults(std::vector<Question>& questions)
{
 int total = 0; //Total score.
 //Keep track of score.
 for (size_t i = 0; i < s_numQuestions; ++i)
 {
 total += questions[i].askQuestion(i + 1);
 }
 //Print Total score.
 clearScreen();
 if (total >= s_failingGrade) {
 std::cout << "\n\n";
 std::cout << "You scored " << total << " out of 100!\n";
 PrintArt(std::ifstream("quiz_passed.txt"));
 }
 else
 {
 std::cout << "You scored " << total << " out of 100....\n";
 std::cout << "Sorry, you failed... Better luck next time.\n";
 PositionCursor();
 }
}
void clearScreen()
{
 std::cout << std::string(22, '\n');
}
void PositionCursor()
{
 std::cout << std::string(22, '\n');
}

• Toby Speight: Here's something of interest if you're randomizing questions and/or answers - Stack Overflow Developer Survey 2018 - Methodology: "The questions were organized into several blocks of questions, which were randomized in order. Also, the answers to most questions were randomized in order."

Answer 1

Here are some things that may help you improve your code.

Use only required #includes

The code has the line:

#include <sstream>

but as far as I can see, nothing from <sstream> is actually used or needed. Only include files that are actually needed.

Consider better naming

The function named InitializeQuizGame is actually rather misleading because it doesn't simply initialize the game, but also runs it to completion. Most of the other names are not bad, but the odd mix of capitalization (as with InitializeQuizGame and load) makes the code a bit harder to read than it should be.

Let the computer do the mathematics

If you decided to make a 20 question quiz or a 50 question quiz, it would be nice if the program would automatically adjust to that without having to recompile the program. Fortunately, it's quite easy to do. Assuming the questions are all weighted the same, and if there are \$n\$ questions, each question is worth \$n/100\$ percent. Since you're reading all of the questions into a std::vector anyway, why not just let the computer perform the mathematics rather than hardcoding the values?

When is a failing grade not a failing grade?

If I see a variable named failingGrade I would expect it to actually represent a failing grade, but surprisingly in this code, if one gets 60% correct (even though s_failingGrade = 60) the message says that I passed the quiz.

Perform input sanitation

The function that reads in the questions does not validate the data. In particular, the correct answer is apparently always supposed to be a, b, c, or d but the data read is not validated to assure that.

Prefer std::istream to std::ifstream

In several cases, a passed parameter is an std::ifstream but the interface would be more flexible if they instead used a std::istream. I'd also recommend having the functions accept a std::istream & to both avoid making a copy and to have the calling function handle any problems opening files. It also makes it slightly easier to write automated tests because you can use std::stringstreams as inputs.

Consider more effective use of objects

The Question class is not a bad start for a quiz program, but the object is a little strange in that its only member function relies on both an externally provided question number and various global variables in order to function. These both suggest to me that it might be better to introduce a Quiz class to encapsulate the vector of Question objects. It might look something like this:

class Quiz {
public:
 class Question {
 public:
 friend std::istream& operator >> (std::istream& is, Question& ques);
 bool ask() const;
 private:
 std::string question_text;
 std::string answer_1;
 std::string answer_2;
 std::string answer_3;
 std::string answer_4;
 char correct_answer;
 };
 Quiz(std::istream &data);
 void operator()() const;
private:
 std::vector<Question> questions;
 static const int s_failingGrade = 60;
 static const char* s_winMessage;
 static const char* s_loseMessage;
 static const char* s_promptAnswer;
};
const char* Quiz::s_winMessage = "Correct!\n";
const char* Quiz::s_loseMessage = "Incorrect, the correct answer was ";
const char* Quiz::s_promptAnswer = "What is your answer?\n";

Usage might look like this:

int main()
{
 std::ifstream in{"quiz_data.txt"};
 if (!in) {
 std::cout << "Error: could not open quiz data file\n";
 return 1;
 }
 Quiz quiz(in); //Load questions from .txt file
 quiz();
}

Avoid hardcoding values

The file names, the failing grade and the various error messages are all hardcoded now. Rather than hardcode those, you could read them from a configuration file, making the program much more flexible and usable at very little additional effort.

Consider shuffling the answers

Right now each question is asked with all of the answers in the same order as they appeared in the input file. It might make for a better quiz if the program shuffled the available answers each time, just as it shuffles the questions.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.

• user33306: One thing about using return statements. I find they make excellent no op statements when using an interactive debugger, since breaking on the curly brace doesn't let one see the values inside the function.
• Edward: Indeed, there are all kinds of little tricks that are useful for debugging, (I use while(1){} for a debugger stop) but they're usually best removed in production code.
• Toby Speight: If you're shuffling the answers, you can require that the first answer (in the data file) is always the correct one (and have InitializeQuestion mark it as correct before shuffling). That means less data to enter, and a lower risk of error.