A Trivia game with SFML

So I've been making a trivia game with SFML and C++ for quite a while now for my school project and I would like to get some constructive criticism, anything helps from best practices to overall correctness.

Everything on GitHub for readability

Here goes main.cpp:

#include "Trivia.hpp"

int main()
{
    Trivia trig;
    trig.Run();
}

Trivia.cpp:

```
#include "Trivia.hpp"
#include
#include
#include

static std::mt19937_64 rngEngine(std::chrono::high_resolution_clock::now().time_since_epoch().count());

Trivia::Trivia()
:triviaWindow(sf::VideoMode(1280, 720), "Trivia! :o")
{
if(!font.loadFromFile("Assets/DejaVuSansCondensed-Bold.ttf")) //If font won't load, output error into console
{
std::cerr 2)
{
triviaWindow.clear(sf::Color(25,25,25));
triviaWindow.draw(gameOverText);
triviaWindow.draw(gameOverText);
triviaWindow.display();

ProcessEvents();
if(event.key.code == sf::Keyboard::Y)
{
GameOver(true);
score=0;
}
else if(event.key.code == sf::Keyboard::N)
{
GameOver(false);
}
}
}
}

std::string Trivia::PointsCollector(int points)
{
//Score is a string
score += points;
QuestionHandler();
TextStringHandler();

return std::to_string(score);
}

int Trivia::RngHandler(int from, int to)
{
std::uniform_int_distribution<> randumb(from, to); //Uniformly distributes anything you put into the function, has from and to value
return randumb(rngEngine);
}

void Trivia::Draw() //Draws Assets
{
triviaWindow.clear(sf::Color(25,25,25));
triviaWindow.draw(textScore);
triviaWindow.draw(questionText);
triviaWindow.draw(answerText);
triviaWindow.draw(heart);
triviaWindow.display();
}

void Trivia::GameOver(bool pick)
{
if(pick)
{
badGuessCount=0;
textScore.setString("Score: " + std::to_str

This looks pretty good! I think it's a great start and the code is pretty clean and easy-to-read. Here are a few suggestions:

Separation of Concerns

I think a single class that does everything is a little confusing. I see 2 different things going on in the code:

• Maintaining and updating the game state

• Displaying the current state and processing events

So I recommend breaking this up into 2 classes: a window class and a game class. This is known as Separation of Concerns. It allows you to change or replace parts of the code without affecting the other parts of the code. And it keeps things separated by functionality. (If this were more complex I might recommend a third class that sends messages between the other 2, but that's overcomplicated for this case, I think.)

If you had a TriviaWindow class and a Trivia class, your main() function would look something like this:

#include "Trivia.hpp"

int main()
{
    TriviaWindow window;
    Trivia trig;
    trig.setWindow(&window);
    trig.Run();
}

I'd put all of the window, text, sprite, etc. members into the window class. And all of the game state into the Trivia class. That way, if you decide you want to display your game differently, you don't have to touch the logic at all. You just write a new TriviaWindow class (or subclass), and use that instead. For example, if you wanted to change the hearts to stars, or you wanted send the text over the network and get responses back from the network, or whatever.

However, doing that will change other things a little bit. As you can see above, the Trivia class now needs a pointer to the TriviaWindow class. Another change is that event processing is now in the window class, so it needs to return information about whether and how it processed events.

I'd create a enum like this:

typedef enum EventResponse {
    EVT_HANDLED = 0, // Some non-game event like window resized, etc.
    EVT_PRESSED_1 = 1,
    EVT_PRESSED_2 = 2,
    EVT_PRESSED_3 = 3,
    EVT_PRESSED_Y = 4,
    EVT_PRESSED_N = 5
} EventResponse;

So Trivia::Run() would do something like this:

void Trivia::Run() //Calls rest of the game functions
{
    QuestionHandler();
    TextStringHandler();
    while(window.isOpen())
    {
        if(badGuessCount<=2)
        {
            Draw();
            EventResponse evtResult = ProcessEvents();
            switch (evtResult)
            {
                case EVT_PRESSED_1:
                    AnswerDetector(1);
                break;

                case EVT_PRESSED_2:
                    AnswerDetector(2);
                break;

                case EVT_PRESSED_3:
                    AnswerDetector(3);
                break;
            }
            Lives(badGuessCount);
        }
        else
        {
            window.SetGameOver();

            EventResponse evtRslt = ProcessEvents();
            if(evtRslt == EVT_PRESSED_Y)
            {
                GameOver(true);
                score=0;
            }
            else if(evtRslt == EVT_PRESSED_N)
            {
                GameOver(false);
            }
        }
    }
}

So you can see that you'll need a couple of additional methods in the TriviaWindow class. You'll also need some public setters to set the text fields. Probably something like:

void setQuestionText(const std::string& newQuestionText);
void setAnswerText(const std::string& newAnswerText);
void setScore(const std::string& newScore);
void setLives(const int numLives);
void setGameOver();

This way, each class does it’s one thing. The window class handles the user - displaying the text and hearts, and getting input from the user, and the Trivia class just handles keeping track of questions and scores.

Handling Construction Failures

I notice in your constructor for Trivia that if anything goes wrong, you simply close the window and return. This is dangerous because what happens is you end up with a half-constructed object. The caller doesn't know that it didn't complete construction, and thinks it's a valid object but it's not. You have 2 choices:

• Use exceptions to tell the caller that something went wrong

• Construct those objects outside of the constructor and pass them in

Exceptions are a big subject and different programmers have very strong feelings about how and when to use them. It’s easier to construct the objects that might fail and pass them in if it goes OK, so let’s do that.

So main() becomes:

```
#include "Trivia.hpp"

int main()
{
sf::Font font;
if(!font.loadFromFile("Assets/DejaVuSansCondensed-Bold.ttf")) //If font won't load, output error into console
{
std::cerr << "Could not load font.\n";
return 0; // Indicates failure
}
sf::Texture hearts[3];
for(int i=0;i<3;++i)
{
if(!heartTextures[i].loadFromFile("Assets/HeartState"+std::to_string(i+1)+".png"))
{
std::cerr << "Couldn't load HeartTexture" + std::to_string(i+1);

StackExchange Code Review Q#84823, answer score: 4