Master engine class for a Pong game

Question

Question: Is having a master class bad practice? I am making a simple pong game, but I'm writing the code as if I were making for something that might be for something more complicated I want to create in the future. Also, am I not doing it in a good manner?

What I am doing: I have a class, in this case "engine", which basically holds everything and runs all of its referenced objects logic inside of its own functions. All of the other objects are then linked to each other by holding the data of the "world or engine" object and use it as a linker to other objects that it can interact with.

NOTE: The sfml media library was used with this project.
note: if you need the ball definitions i'll post them

#include <SFML/Graphics.hpp>

#include <vector>
#include <iostream>
#include <stdlib.h>

class paddle;
class ball;
class enemy;
class player;


class engine{
    ball *gameBall;
    enemy *enemyPaddle;
    player *playerPaddle;
    Clock gameClock;
    RenderWindow &refWindow;
    int gameSpeed;
    Event windowEvent;

    void eventDraw();
    void eventUpdate();
    void handleWindow();

public:
    engine( RenderWindow &rendy);
    ~engine();
    bool start();
    Vector2f getWindowSize();
    void drawPart(Drawable &art);
    Time getGameClock(){ return gameClock.getElapsedTime(); }
};


class ball{
private:
    CircleShape myShape;
    engine& world;
    int radius;
    Color color;
    Vector2f position, velocity;
    short signed int dirX, dirY;
    short signed int getDirection();
    void wallBounce();
public:
    ball( engine& gameEngine);
    void draw();
    CircleShape getCircle(){ return myShape; }
    void pop();
    void update();
};

int main()
{
    RenderWindow window(VideoMode(640, 480), "Pong");
    engine gameEngine( window );
    return (int)gameEngine.start();
}


// Class Prototyes
    // Engine
        engine::engine(RenderWindow &rendy):refWindow(rendy){
            gameBall = new ball(*this);
            enemyPaddle = new enemy;
            playerPaddle = new player;
            gameClock = Clock();
            gameSpeed = (1000/60.0);
            windowEvent = Event();
        }
        engine::~engine(){
            delete enemyPaddle;
            delete playerPaddle;
            delete gameBall;
        }

        void engine::handleWindow(){
            if ( refWindow.pollEvent( windowEvent ) )
            {
                 if ( windowEvent.type == Event::Closed )
                     refWindow.close();
            }
        }

        bool engine::start()
        {
            while( refWindow.isOpen() )
            {

                while( gameClock.getElapsedTime().asMilliseconds() > gameSpeed )
                {
                    eventUpdate();
                    eventDraw();

                    // Restart Clock
                        gameClock.restart();
                }

                handleWindow();
            }

            return true;
        }

        void engine::eventUpdate(){
            gameBall->update();

        }
        void engine::eventDraw(){
            refWindow.clear( Color::Black );

            gameBall->draw();

            refWindow.display();
        }

        Vector2f engine::getWindowSize(){
            Vector2f tempVec;
            tempVec.x = refWindow.getSize().x;
            tempVec.y = refWindow.getSize().y;
            return tempVec;
        }

        void engine::drawPart(Drawable &art){
            refWindow.draw( art );
        }

Answer 1

A "master class" is a very close relative to a singleton or a global. It can easily be a design which violates the Dependency Injection principle, and leaves you with non-modular code. In this case your engine is also violating the Single Responsibility principle, as it's doing a lot of unrelated tasks: it's handling windows, providing the game's main driver, moving the ball, drawing the screen, and hosting all kinds of different data.

Try this task: write a unit test that exercises just the engine::handleWindow() routine without needing to create a real ball, enemy, or player. It's hard, because your constructor builds all of those objects.

You've already got a start. For example, you can easily write a unit test that tests engine::eventDraw without creating a real RenderWindow, because you can pass a mock RenderWindow on the engine constructor. You're using dependency injection.

As you're going to have a singleton in main (and every program does at some level), I recommend you try to keep that singleton as thin as possible.

It's not that your existing code isn't going to work, but it's going to be a problem to maintain, especially as you extend it to add functionality. Where does scorekeeping come in? Player preferences? High score table? Head-to-head play with two people? The more you want to add to this, the more you're going to need to change engine, and the less confident you're going to be that you aren't breaking something else.

Answer 2

John Deters did a great job answering this; I just have a couple more comments I'd like to make:

You can start off by creating an EntityManager class that you put all your game entities in (player, ball, enemy paddle). This will take out the

ball *gameBall; 
enemy *enemyPaddle; 
player *playerPaddle;

code, and instead, you could have

EntityManager m_entityManager;

instead.

Also, a bit of style that I've always found helpful (I know that style differs between every programmer), but adding the "m_" prefix to member variables helps to distinguish between local variables and class data.

//for example
//instead of:
Clock gameClock;

//you could have
Clock m_gameClock;
//or
Clock mGameClock;

the point is that you're reminding yourself that you're working on member variables rather than local variables/function parameters.

Answer 3

You can break up your engine class based upon the responsibilities you have given it.

Namely: Draw Update HandleWindow

These tasks can be handled by separate modules. You may at some point decide to build a graphics engine to handle the Draw aspect of the game, a physics or logic engine to handle the Update aspect of the game, and potentially an input engine or UI engine to handle the HandleWindow portion of the game. This would allow you to update the graphics engine in the future (maybe implementing a 3D version of pong) without needing to change any other piece of the engine. You could add logic to handle collision with power-up items in the future in the logic engine of your game with a minimal impact on how the graphics are actually rendered or the input is managed. You could also then add support for different input devices such as a joystick without having any impact at all on the graphics system or the game logic.

Ultimately, the engine could be responsible solely for telling the game to run and delegate all other responsibilities to systems that are better equipped to handle the specifics.

Finally, when you decide to make a game that is not Pong any more, all you have to do is switch out the appropriate subsystems to make a new game. For example, if you wanted to make a breakout game, the UI and graphics systems might be identical to your pong game and reused, but the logic system would be switched out to support logic for breakout.