C++/SDL2 "Space Invaders"

My biggest issue in this project was the structure of the code and using the necessary data structures. I organized it to the best of my ability but I am sure there much more room improvement and I would love some advice on this. The way I have my enemies firing feels a bit hacky and I'm not sure as to how to have that in a better way.

Also, I am not entirely sure why I have to have #include in a header or .cpp file at times. This is dependent in certain classes and I am unsure why.

main.cpp

#include "Display.h"
#include "Game.h"
#include "Media.h"
#include "Player.h"

#include 
#include 

int main(int argc, char* args[])
{
    Display display;
    Game game;
    Media media;
    Player player;

    game.start(display, media, player);

    printf("\n\n\nYou Win!");
    SDL_Delay(3000);

    return 0;
}

Display.h

#pragma once
#include 
#include 
struct Laser;
struct Enemy;
class Player;
class Media;
class Display
{
public:
    Display();
    ~Display();

    bool init();
    void render(Media& media, Player& player, std::vector& enemies, std::vector& playerLasers, std::vector& enemyLasers);

    SDL_Surface* getWindowSurface() { return m_windowSurface; }
    SDL_Renderer* getRenderer() { return m_renderer; }
protected:

private:
    SDL_Window* m_window = nullptr;
    SDL_Surface* m_windowSurface = nullptr;
    SDL_Renderer* m_renderer = nullptr;

    //Screen resolution
    const int SCREEN_WIDTH = 800;
    const int SCREEN_HEIGHT = 800;

};

Display.cpp

```
#include "Display.h"
#include "Media.h"
#include "Player.h"
#include "Enemy.h"
#include "Laser.h"

#include
#include

Display::Display()
{
//init();
}

Display::~Display()
{

}

bool Display::init()
{
bool success = true;
//Initialize SDL Video
if (SDL_Init(SDL_INIT_VIDEO) & enemies, std::vector& playerLasers, std::vector& enemyLasers)
{
SDL_RenderClear(m_renderer);
SDL_SetRenderDrawColor(m_renderer, 255, 255, 255, 0);

//Render player
SDL_RenderCopy(m_r

This is a fairly long question, so this is by no means a thorough review.
I'll be focusing on the more obvious things that we can get out of the way right now :)

main

Your main function is clean and I specially like to see that you're passing dependencies through function parameters.

The printf+delay seems a bit misplaced though, it could be done inside the game class. Eventually you will probably want to draw the message to screen instead of printing to the console, so if you move that into the game, main can be left unchanged.

return 0 at the end of main is optional. This is a special rule that only applies to the main function. If the program reaches the end of main without encountering a return statement, it is the same as returning zero.

Display class

You could clean it up a bit, get rid of unnecessary stuff. The default empty constructor doesn't have to be provided when there's no work to be done in it. Let the compiler supply a default for you.

The protected section is also empty, so omit that tag. Same for the Media class.

SDL shutdown: Where is it happening? I would expect the destructor of Display to be destroying the SDL objects it created in init().

Also, the work done by init should be done by the constructor, to avoid having
an invalid or partly constructed object. I suppose you created init() to be able to return an error from the function, but you could have the constructor throw an exception instead. Having explicit init/shutdown methods in a class is usually not a great design, because you can have a partially constructed object. Instead, take advantage of constructors and destructors and use the RAII style whenever possible.

These two constants should be marked static, alternatively, use static constexpr if your compiler has descent C++11 support:

const int SCREEN_WIDTH = 800;
const int SCREEN_HEIGHT = 800;

They are still taking unnecessary space in the class (the size of an int each). If you mark them as statics, then the compiler will optimize them to compile-time constants and they will take no extra space in the objects.

static constexpr int SCREEN_WIDTH = 800;
static constexpr int SCREEN_HEIGHT = 800;

There are other such instances in the Player class.

Make thorough use of const for parameters. E.g.:

void Display::render(Media & media, Player & player, std::vector& enemies, std::vector& playerLasers, std::vector& enemyLasers)

When you take read-only parameters by pointer of reference, be sure to mark them as const. This will prevent you from accidentally modifying stuff that shouldn't be and will also convey that info to the caller, which will know the object is not changed by the function.

Display::render() won't change any of the vectors nor the player, so the expected signature would be:

void Display::render(Media & media, const Player& player, 
                     const std::vector& enemies, 
                     const std::vector& playerLasers, 
                     const std::vector& enemyLasers)

Media class

Has the same issue of not using RAII and not releasing the resources it allocates. You must free the textures in the destructor to avoid leaking them!

Miscellaneous

-
Use stderr (std::cerr) to report errors. Using std::cout/printf outputs to stdout, which is not the expected place for them. Being consistent with the convention of using stderr for errors allows users to filter the normal output of a program from the errors.

-
Use of rand() is discouraged. It has a very bad rep because implementations are usually very trivial, making the pseudo-random numbers it generates very predictable. Even for something as simple as a game
it might not be suitable. It could cause easily predictable behavior in the game where it should not have. Take a look at the new ` API from C++11 as a future exercise.

-
This is not simulating 60 frame-per-second, it merely slows down the game loop:

//Simulate 60 fps - Read on tutorial, not entirely sure if this is ok. 
 SDL_Delay(16);

On a fast machine, you might not even notice it, but on a slow one, it will
just freeze for 16 milliseconds, potentially bring the frame-rate dangerously below the golden 30fps. I don't see a lot of reasons for doing that, but if you'd like to limit the frame-rate to at most 60 frame-per-second, you have to measure the time taken to render the frame and then sleep for the precise amount to prevent rendering too fast. Something along the lines of:

``
// 60fps => 16ms in a frame
constexpr int MIN_MILLISECONDS_PER_FRAME = 16;

while (true)
{
const int startTime = getTimeMilliseconds();

// do game logic, rendering, etc...

const int endTime = getTimeMilliseconds();
const int millisecondsThisFrame = endTime - startTime;

if (millisecondsThisFrame < MIN_MILLISECONDS_PER_FRAME)
{
// This frame was done in less than 16ms, so we stall for a
// while to en

StackExchange Code Review Q#121117, answer score: 4