Infrastructure for my 2D game using Allegro

I am making a small 2D game with the use of Allegro 5 library. I am making use of inheritance e.g. sprite class as the base class and player class as the child class, but I want to know if it's a good idea to approach it in this way and other ways that are better to do this, if there are any.

Main.cpp

#include "Allegro.h"

int main()
{
    Allegro *allegro = new Allegro();
    allegro->init();
    allegro->createWindow(60.0, 640, 480);
    allegro->gameLoop();

    return 0;
}

Allegro.h

#ifndef ALLEGRO_H_
#define ALLEGRO_H_

#include 
#include 

#include "Player.h"
#include "Keyboard.h"

class Allegro
{
private:
    ALLEGRO_DISPLAY *display;
    ALLEGRO_TIMER *timer;
    ALLEGRO_EVENT_QUEUE *event_queue;

    Keyboard keyboard;
    Player player;

    bool looping, redraw;

public:
    Allegro();
    ~Allegro();

    int init();
    int createWindow(float FPS, int w, int h);
    void gameLoop();
};

#endif

Allegro.cpp

```
#include "Allegro.h"

Allegro::Allegro()
{
display = NULL;
timer = NULL;
event_queue = NULL;

looping = true, redraw = false;
}

Allegro::~Allegro()
{
al_destroy_event_queue(event_queue);
al_destroy_timer(timer);
al_destroy_display(display);
}

int Allegro::init()
{
if (!al_init())
{
return -1;
}

return 0;
}

int Allegro::createWindow(float FPS, int width, int height)
{
display = al_create_display(width, height);
if (!display)
{
al_destroy_display(display);
return -1;
}

timer = al_create_timer(1.0 / FPS);
if (!timer)
{
al_destroy_timer(timer);
al_destroy_display(display);
return -1;
}

event_queue = al_create_event_queue();
if (!event_queue)
{
al_destroy_event_queue(event_queue);
al_destroy_timer(timer);
al_destroy_display(display);
return -1;
}

al_install_keyboard();
al_init_image_addon();

al_register_event_source(event_queue, al_get_display

Style

I find that you have a consistent and by many acceptable style. This is muy bueno. Some people (myself included) prefer some kind of prefix on member variables (for example m_) to make them stand out in the absence of fancy syntax highlighting.

A personal preference of mine is to always have public first, followed by protected and then private. For the reason that you should write code for those who are going to use it. And those who are going to use it are interested in the public API, not the implementation details. So that's the first thing they should see when they look at the header.

Other than that, there isn't really anything to complain over.
Naming

I find the class name Allegro to be a bit on the bad side as it doesn't as much refer to what the class does, but rather what technology it uses. I would much prefer Application.
Include Guards

Although not in the official standard, many (if not most) compilers support the #pragma once extension instead of the classical include guard you are using. There are some benefits to using #pragma once and it might be worth using them instead.
Init methods are a code smell

The presence of init type methods is a indicator of poor design. Where possible, classes should be constructed as close as possible to their final state.

This avoid issues where you forget to call the init method, or worse forget to call the init methods in the right order. Or call it twice...

In your case the constructor of Allegro should do the equivalent of Allegro::init() and Allegro::createWindow(...) combined, it should obtain all resources necessary to use the class. And of course your destructor should do the inverse, free all acquired resources. This is also referred to as Resource Acquisition Is Initialization (RAII) and is a generally desirable pattern.

The same goes for the Sprite class, the class isn't usable without first calling setBitmap so the constructor should do the equivalent work of setBitmap. Also I do believe that you never free the bitmap (the destructor is empty).
Accessors (setters/getters) are a code smell

If you have a setX and getX method for a variable in your class, you might as well make the variable public because you have already broken encapsulation. But we know that public variables are bad, so ergo set/get methods are also bad.

Or that's how I reason and this guy agrees with me.

When you see the presence of a set or get method you have to stop and think:

Okay why is some one manipulating the internal state of this class directly? Can't that logic be moved to this class instead?

Implement the rule of three

You really should implement the Rule of three for all your classes that manage resources to avoid subtle bugs.
The Keyboard class

This class puzzles me, I find it hard to motivate its existence as a class as it only has one public member array of booleans.

I would move the logic from the game loop that updates the keyboard state into the keyboard class. The make the key array private and add methods to query whether or not the appropriate key is pressed. While you're at it you can move the al_install_keyboard(); call there too, make the class earn its keep.

Finally, although it may be overkill for your simple game it may be worth looking into the Command Pattern which I find simplifies application/game logic significantly when you start doing anything non-trivial.

StackExchange Code Review Q#95850, answer score: 7