1
\$\begingroup\$

I would like to enhance my keyboard class to be more tight and encapsulated. The first issue I've addressed is Singleton design pattern. I've done this as follows:

/*** inside keyboard.h ***/
class Keyboard
{
public:
    static Keyboard* Instance(){
        if(s_pInstance == NULL)
            s_pInstance = new Keyboard;
        return s_pInstance;
    }
    ...
private:
    static Keyboard* s_pInstance;
    Keyboard() {}
    ...
};

/*** inside keyboard.cpp ***/
#include "keyboard.h"

Keyboard* Keyboard::s_pInstance = NULL;
...

The design pattern works just fine. Now in while(true) I can do

while(Window is Open){
...
    if ( Keyboard::Instance()->isKeyDown() ){
        if ( Keyboard::Instance()->m_key == GLFW_KEY_E )
            std::cout << "E is pressed ..." << std::endl;
    }
...
}

In GLFW, I have to declare key_callback as static member function. To store keyboard actions, I've introduced global variables and every time I call any Keyboard function, I need to update those global variables as follows:

bool Keyboard::isKeyDown()
{
    // assign global variables to data members 
    update(g_key, g_scancode, g_action, g_mode);

    if( m_action == GLFW_PRESS ){
        std::cout << "Key down..." << std::endl;
        ...
        // reset global variables 
        m_action = -1;
        g_action = -1;
        return true;
    }


    return false;
}

void Keyboard::update(int k, int s, int a, int m)
{
    m_key      = k;
    m_scancode = s;
    m_action   = a;
    m_mode     = m;
}

The code works but I don't believe this is the right way to encapsulate my class. Any suggestion how to remove these global variables which is a consequence due to using static function?

This is the full code

#ifndef _KEYBOARD_H
#define _KEYBOARD_H

#include <GLFW/glfw3.h>

class Keyboard
{
public:


    static Keyboard* Instance(){
        if(s_pInstance == NULL)
            s_pInstance = new Keyboard;
        return s_pInstance;
    }
    void init(GLFWwindow* window);
    void update();
    bool isKeyDown();
    bool isKeyUp();

    int m_key, m_scancode, m_action, m_mode;
private:
    static void key_callback(GLFWwindow* window, int key, int scancode, int action, int mode);

    GLFWwindow* m_pWindow;
    Keyboard() {}
    static Keyboard* s_pInstance;
    bool keys[1024]; 

    void update(int, int, int, int);

};

#endif

keyboard.cpp is

#include "keyboard.h"
#include <iostream>

Keyboard* Keyboard::s_pInstance = NULL;

int g_key, g_scancode, g_action, g_mode;


void Keyboard::init(GLFWwindow* window)
{
    m_pWindow = window;
    glfwSetKeyCallback(window, key_callback);
}

void Keyboard::key_callback(GLFWwindow* window, int key, int scancode, int action, int mode)
{
    g_key      = key;
    g_scancode = scancode;
    g_action   = action;
    g_mode     = mode;
}



bool Keyboard::isKeyDown()
{
    update(g_key, g_scancode, g_action, g_mode);


    if( m_action == GLFW_PRESS ){
        std::cout << "Key down..." << std::endl;
        keys[m_key] = true; 


        m_action = -1;
        g_action = -1;
        return true;
    }


    return false;
}

bool Keyboard::isKeyUp()
{
    update(g_key, g_scancode, g_action, g_mode);


    if( m_action == GLFW_RELEASE ){
        std::cout << "Key up..." << std::endl;
        keys[m_key] = false; 

        m_action = -1;
        g_action = -1;
        return true;
    }


    return false;
}

void Keyboard::update(int k, int s, int a, int m)
{
    m_key      = k;
    m_scancode = s;
    m_action   = a;
    m_mode     = m;
}
\$\endgroup\$
1
\$\begingroup\$

In your key_callback() function, you can get the Keyboard singleton and directly set its m_key, m_scancode, m_action and m_mode member variables. Something like this:

void Keyboard::key_callback(GLFWwindow* window, int key, int scancode, int action, int mode)
{
    Keyboard::instance->update(key, scan code, action, mode);
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Perfect. Thank you. However, since I need to reset these variables this means the variables need to be accessed from multiple threads, I've declared them as std::atomic<int> m_key \$\endgroup\$ – CroCo May 26 '17 at 5:59
  • \$\begingroup\$ On most operating systems I've used, UI events only occur on the main UI thread, so you'd only be setting these values on the main UI thread if you're doing it from an event handler. You can read on other threads without issue if they're all atomic. Though if you're reading more than one value, there's a chance you could get the key from the previous event, say, and the scan code from the current event. You could always protect them with a mutex or other type of threading construct. \$\endgroup\$ – user1118321 May 26 '17 at 15:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.