Good way to structure a SDL project

Question

I am absolutely new to c++ and am trying to learn SDL2 with c++. I am setting up my first project and I was unsure if the way I have structured the project is good or if I have over done it. Here is the code:

generalFuncs.h

#pragma once
#include "SDL.h"
#include <iostream>
struct State
{
 bool quit = false;
};
class CreateWindow
{
public:
 CreateWindow(
 int x,
 int y,
 int w,
 int h,
 bool fullScreen = false
 )
 {
 pWindow = SDL_CreateWindow("", x, y, w, h, fullScreen);
 }
 inline SDL_Window* getWindow()
 {
 return pWindow;
 }
private:
 SDL_Window* pWindow;
};
class Events
{
public:
 void handleEvents(State* state)
 {
 while (SDL_PollEvent(&event))
 {
 if (event.type == SDL_QUIT)
 state->quit = true;
 }
 }
private:
 SDL_Event event;
};

renderer.h

#pragma once
#include "SDL.h"
class Renderer
{
public:
 Renderer(SDL_Window* pWindow)
 {
 pRenderer = SDL_CreateRenderer(pWindow, -1, 0);
 }
 inline SDL_Renderer* getRenderer()
 {
 return pRenderer;
 }
 inline void clearScreen(int r, int g, int b, int a)
 {
 SDL_SetRenderDrawColor(pRenderer, r, g, b, a);
 SDL_RenderClear(pRenderer);
 }
 inline void update()
 {
 SDL_RenderPresent(pRenderer);
 }
 void render()
 {
 
 }
private:
 SDL_Renderer* pRenderer;
};

main.cpp

#include <iostream>
#include <SDL.h>
#include <vector>
#include "generalFuncs.h"
#include "renderer.h"
int main(int argc, char* argv[])
{
 if (SDL_Init(SDL_INIT_EVERYTHING) != 0)
 {
 std::cout << "Could not initialze SDL properly \n" << SDL_GetError() << std::endl;
 return 0;
 }
 CreateWindow window(SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 1200, 600);
 Renderer renderer(window.getWindow());
 State state;
 Events events;
 if (window.getWindow() == NULL)
 {
 std::cout << "Could not window properly \n" << SDL_GetError() << std::endl;
 return 0;
 }
 if (renderer.getRenderer() == NULL)
 {
 std::cout << "Could not window properly \n" << SDL_GetError() << std::endl;
 return 0;
 }
 while (!state.quit)
 {
 events.handleEvents(&state);
 renderer.clearScreen(255, 255, 255, 255);
 renderer.render();
 renderer.update();
 }
 SDL_DestroyWindow(window.getWindow());
 SDL_Quit();
 return EXIT_SUCCESS;
}

The way I plan on using this is make other header files, include them to render.h, then draw them in the render method. Please let me know if there are better way to do things or if my setup can be improved. Thank you.

Answer 1

I think that yes, the code can be structured somewhat better.

First of all, I'd tend to "RAII all the things". Code like this at the end of main makes me extremely suspicious:

 SDL_DestroyWindow(window.getWindow());
 SDL_Quit();

I'd strongly prefer that those be handled in the destructor of some object so they happen automatically.

So, I'd rename CreateWindow to just Window. Then I'd add a destructor that called SDL_DestroyWindow, so it'll automatically clean itself up when it goes out of scope.

class Window {
public:
 Window(int x, int y, int w, int h, bool fullScreen = false) {
 pWindow = SDL_CreateWindow("", x, y, w, h, fullScreen);
 if (pWindow == nullptr)
 throw std::runtime_error("Unable to create window");
 }
 ~Window() { SDL_DestroyWindow(pWindow); }
 operator SDL_Window*() { return pWindow; }
private:
 SDL_Window *pWindow;
};

Likewise I'd add an Application class (or something on that order that calls SDL_Init in its ctor, and SDL_Quit(); in its dtor, so simply defining a variable of that type automatically initializes SDL, and cleans up before it goes out of scope.

class Application {
public: 
 Application() { 
 if (SDL_Init(SDL_INIT_EVERYTHING) != 0)
 {
 std::stringstream buffer;
 buffer << "Could not intialize SDL properly: " << SDL_GetError();
 throw std::runtime_error(buffer.str());
 } 
 }
 ~Application() {
 SDL_Quit();
 }
};

...and of course, Renderer should clean up after itself as well:

 ~Renderer() { 
 SDL_DestroyRenderer(pRenderer);
 }

I'd also add definitions of copy assignment, move assignment, copy construction, and move construction to Renderer, Window and Application, so somebody won't accidentally do something nasty and breaking with them. In this case, they're probably move-only classes, so we'd do something on this order:

 Renderer(Renderer const &) = delete;
 Renderer(Renderer &&other) { 
 pRenderer = other.pRenderer; 
 other.pRenderer = nullptr; 
 }
 Renderer &operator=(Renderer const &) = delete;
 Renderer &operator=(Renderer && other) { 
 pRenderer = other.pRenderer; 
 other.pRenderer = nullptr; 
 return *this;
 }

Window would probably be similar in this regard, and Application should probably delete all of them.

I'd also have each of the classes report problems with construction by throwing an exception, rather than leaving it to client code to detect the problem when (for example) it has failed to create a window.

 Window(int x, int y, int w, int h, bool fullScreen = false) {
 pWindow = SDL_CreateWindow("", x, y, w, h, fullScreen);
 if (pWindow == nullptr)
 throw std::runtime_error("Unable to create window");
 }
}

Rather than Events::handleEvents being passed a Boolean in a structure, I'd have it just return a value to indicate when the program should exit:

 bool handleEvents()
 {
 while (SDL_PollEvent(&event))
 {
 if (event.type == SDL_QUIT)
 return false;
 }
 return true;
 }

I'd also put all of these in a namespace named something like sdl (or sdl2, or something on that order).

With that "stuff" in place, our main can shrink to something on this order:

int main(int argc, char* argv[])
{
 try {
 sdl::Application app;
 sdl::Window window(SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 1600, 1200);
 sdl::Renderer renderer(window);
 sdl::Events events;
 while (events.handleEvents())
 {
 renderer.clearScreen(255, 255, 255, 255);
 renderer.render();
 renderer.update();
 }
 }
 catch(std::exception const &e) { 
 std::cerr << e.what() << "\n";
 }
}

Although I haven't shown it above, I'd at least be tempted to require that a reference to an sdl::Application be passed to the ctor when you create an sdl::Window object. This assures that you've created an sdl::Application object first, thereby assuring that SDL is initialized before you try to create a window with it.

• Apple_Banana: Thank you very much. Appreciate it. I have one more question: What is the point of "copy assignment, move assignment, copy construction, and move construction". Pardon my ignorance, I am new.
• Apple_Banana: Is operator SDL_Window*() { return pWindow; } operator overloading ? In tutorials I have only seen overloading with ints and other builtin data structures.