HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

SDL Input Managment

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
inputmanagmentsdl

Problem

I've created an SDL input manager for my 3D OpenGL applications. It's not finished yet, but I would like to know if I'm doing it the right way.

I really tried to optimise and compact my code, as it is for an OpenGL library.

InputHandler.h

#pragma once
#include 
#include 
#include 
#include "SDL_Handle.h"

#define TheInputHandler InputHandler::Instance() //Yes... Macros aren't good... but we can't ude typdef, or anything else :)

class InputHandler
{
    public:
        static InputHandler& Instance(){
            static InputHandler instance;
            return instance;
        }

        void Update();
        inline bool ExitRequested(){ return _hasExit; }
        inline glm::uvec2 GetMousePosition(){ return _mousePosition; }
        inline bool KeyDown(SDL_Scancode key){
            return _keyStates != 0 ? _keyStates[key] : false;
        }

        ///disable assignemant operators
        InputHandler(InputHandler const&) = delete;
        InputHandler& operator=(InputHandler const&) = delete;
    private:
        InputHandler();/// singleton pattern
        virtual ~InputHandler();
        SDL_Handle _handle;
        bool _hasExit = false;
        glm::uvec2 _mousePosition;
        const Uint8* _keyStates = nullptr;
};


InputHandler.cpp

InputHandler::InputHandler(){
    _handle = SDL_Handle(); /// init SDL : uses RAII to avoid initializing twice !
}
InputHandler::~InputHandler(){}

void InputHandler::Update(){
    _keyStates = SDL_GetKeyboardState(0);
    SDL_Event event;
    while(SDL_PollEvent(&event))
    {
        switch(event.type)
        {
            case SDL_QUIT:
                _hasExit = true;
                break;
            case SDL_MOUSEMOTION:
                _mousePosition = glm::uvec2(event.motion.x, event.motion.y);
                break;
        }
    }
}

Solution

Right so you're using a Singleton pattern because you find it reasonable that there will only ever be one input handler. You correctly implement the singleton pattern in C++, which is good. The bad thing is that the pattern in of itself is bad.

A better design is to make InputHandler an interface with virtual functions. Then create a concrete instance SDLInputHandler which implements InputHandler. Then, wherever you need to parse inputs you pass a pointer to a InputHandler. In your main class you can simply have an instance of SDLInputHandler which you pass pointers to.

An added benefit of this is that you can implement an InputHandler that can generate a fixed sequence of inputs, this is very useful for testing your code or for playing back a sequence of user inputs, like a recorded game for example.

If you at this point are thinking:

No way, Jose! I'm not going to be passing that pointer all over my application!

Then that indicates a bigger issue you're having in your overall design. The parts of your application that are directly dealing with input should be quite limited and small. If you indeed are having this phenomena, you should really look into the Command Pattern. It will make your life easier.
edit

If I'm not mistaken, all names that start with an underscore are reserved for the implementation and all your member variable names are technically not allowed.

Context

StackExchange Code Review Q#93968, answer score: 5

Revisions (0)

No revisions yet.