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" "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question 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.
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.
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.
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::sringstreamstringstream
s as inputs.
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::sringstream
s as inputs.
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::stringstream
s as inputs.
Here are some things that may help you improve your code.
Use only required #include
s
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::sringstream
s 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 theexit
function with the value returned by themain
function as its argument; reaching the}
that terminates themain
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.