Function Pointer With Vector

Question

I'm new in C++. I'm trying to do clickable button with OpenGL. I work for add callback function for each button for 2 days, I tried many methods I found but I can't do this. My below codes are giving memory error. Where is my mistake?

main.h

#include <vector>
class Button {
public:
    // Storage Vector
    static std::vector<Button> Buttons;

    // typedef Function 
    typedef void (*pointerFunction)();

    // Constructor
    Button(/*Parameters*/);

    // Setting Callback
    void setCallBack(void(*function)());

    // Callback pointer
    pointerFunction callback;

    int value{ 4 };
};

main.cpp

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

std::vector<Button> Button::Buttons;

Button::Button(/*Parameters*/) {
    // ...
    Button::Buttons.push_back(*this);
}

void Button::setCallBack(void(*function)()) {
    this->callback = function;

    this->callback(); // Here is work!
}

void testFunction() {
    std::cout << "Test\n";
}

void createMember() {
    Button classMember;
    classMember.setCallBack(&testFunction);
}

int main() {
    createMember();

    for (Button& button : Button::Buttons) {
        std::cout << button.value; // I can access this value.
        button.callback(); // But here is give memory error!
    }

    return 0;
}

Answer 1

Your mistake is that you create a local object, push the copy of it into the vector, put the callback address to the original object, and then destroy the original object. Well, you can put the callback address as the constructor argument, then the copy would have it.

// Constructor
Button(void(*function)(), /*Parameters*/) : callback{function} {
    Button::Buttons.push_back(*this);
}

But I would recommend to add a static function to the Button class which is responsible for creation a Button object and returning reference to it. This is also eliminate unnecessary creation/deletion of temporary objects.

#include <iostream>
#include <vector>

class Button {
public:
    // Storage Vector
    static std::vector<Button> Buttons;

    // typedef Function 
    typedef void (*pointerFunction)();

    // Constructor
    Button(/*Parameters*/);

    // Setting Callback
    void setCallBack(void(*function)());

    // Callback pointer
    pointerFunction callback;

    template<class... U>
    static Button& createButton(U&&... u) {
        return Buttons.emplace_back(std::forward<U>(u)...);
    }

    int value{ 4 };
};

std::vector<Button> Button::Buttons;

Button::Button(/*Parameters*/) {
    // ...
    Button::Buttons.push_back(*this);
}

void Button::setCallBack(void(*function)()) {
    this->callback = function;

    this->callback(); // Here is work!
}

void testFunction() {
    std::cout << "Test\n";
}

void createMember() {
    auto &classMember = Button::createButton(/**/);
    //Button classMember;
    classMember.setCallBack(&testFunction);
}

int main() {
    createMember();

    for (Button& button : Button::Buttons) {
        std::cout << button.value; 
        button.callback();
    }

    return 0;
}

Answer 2

Your createMember function don't work as you expect.

void createMember() {
    Button classMember;
    classMember.setCallBack(&testFunction);
}

Creates a local object that will be destroyed at function exit.

You can do it like this (though I don't think it is a good solution.)

Button & createMember() {
    static Button classMember;
    classMember.setCallBack(&testFunction);
    return classMemeber;
}

A better solution:

std::vector<Button> Button::Buttons;

int main() {

    Button b;

    for (Button& button : Button::Buttons) {
        button.setCallBack(testFunction);
        std::cout << button.value; // I can access this value.
        button.callback(); // But here is give memory error!
    }

    return 0;
}

Note that you have to defile Button::Buttons somewhere as it is a static member This was correct in your code, I overlooked it.

And, to add at least a Button, you have to create one to be added to the vector.

Ouput:

Test

4Test

You are calling testFunction twice, at setCallBack and in the loop. (I've added a newline.)

If as the createMember function name suggest, you want to call that to create each new element, you could pass the function pointer in constructor. If it is trivially copyable like is in your example (no pointers or resource allocation in the class) you can just create the instance and the vector copy will be fine.

Button::Button(pointerFunction f) : callback (f) {
    // ...
    Button::Buttons.push_back(*this);
}

void createMember() {
    Button classMember (testFunction);
}

int main() {
    createMember ();

    for (Button& button : Button::Buttons) {
        std::cout << button.value; // I can access this value.
        button.callback(); // But here is give memory error!
    }
}

I don't think this is a good design for anything real, though.

Answer 3

Within this function

void createMember() {
    Button classMember;
    classMember.setCallBack(&testFunction);
}

there are two things that are being doing. The first one is creating the local object classMember. The called construcfor pushes a copy of the object inside the vector Buttons. The data member callback of the copy was not initialized.

It is the data member callback of the .local object classMember that was initialized after its copy was pushed on the vector.

Rewrite the function at least like

void createMember() {
    Button classMember;
    Button::Buttons.back().setCallBack(&testFunction);
}

You should initialize all data members for example using in particular the literal nullptr if a corresponding initializer was not explicitly supplied. In this case you will be able to check whether a data member of a pointer type is equal to nullptr or stores an actual value.