C++ API design: is using void* a bad idea?

Question

I am working on an API that I want as generic as possible on the caller side. Main design idea is to provide a signal/slot sort of implementation that allows the user of the API to subscribe to a given set of events, and attach user-defined callbacks to them.

Public interface looks something like this:
RetCallback subscribe(EventEnum& ev, std::function<void(void*)> fn) const; : note the void(void*) signature here. EventEnum is given in a public header file, as well as types definition.

Inner-works of the API would then notify its subscribed observers of the event through a notify method and provide data to forward to the client :

void dummyHeavyOperation() const {
    std::this_thread::sleep_for(2s);
    std::string data = "I am working very hard";
    notify(EventEnum::FooEvent, &data);
}

Client subscribes and casts data to (documented) type as follows:

auto subscriber = Controller->subscribe(EventEnum::FooEvent, callback);

where

void callback(void* data) {
    auto* myData = (std::string*) data;
    std::cout << "callback() with data=" << *myData << std::endl;

    /// Do things
}

Is this a reasonable design or is this frowned upon? What is your experienced modern C++ developer mind tells you?

[EDIT]
I should add as well that the API is delivered as a shared library loaded at run-time. So any compile time coupling (and code generation for that matter, unless I'm mistaken) is off the table

Thanks!

Answer 1

C++ API design: is using void* a bad idea?

Yes.

To implement equivalent API, you should use std::any, which is a checked version of type erasure. std::any is only available in the standard library since the current, C++17 standard version. If you don't have C++17 (or don't want user of the API to depend on C++17), then you can use a non-standard implementation instead.

An alternative is to not erase the argument type at all, but use templates all the way instead. See Boost Signals for an example of such callback API

Answer 2

Yes, void* is a bad idea. Even more so when the involved types come from you, not the user!

If different events pass different types data to the client, then enforcing type safety is very useful for the user. It prevents things like accidentally passing a callback that expects a string but is called with a double. You want your API to be hard to misuse.

For example, you could do this:

template<class T>
RetCallback subscribe(EventEnum& ev, std::function<void(T)> fn) const;

Subscribers would spell out the type at call site:

auto subscriber = Controller->subscribe<std::string>(EventEnum::FooEvent, callback);

You can then check in subscribe whether the EventNum is ok with that callback signature, or you could even (depending on how many events and callback data types you have) have different EventNum types for each callback data type so that it is impossible to even call subscribe with mismatching event type and callback signature, like this: https://godbolt.org/g/7xTGiM

notify would have to be done in a similar way as subscribe.

This way, any mismatch is either impossible (i.e. compiler-enforced) or caught immediately in your API instead of causing unexpected casting failures later on in user code.

Edit: As discussed in the comments, if pinning the user on compile-time event values is ok, you can even template on the event num itself: https://godbolt.org/g/9NYVh3

Answer 3

Literally anything is better than void*. It's a nasty hack from a time when people were still learning about what people want from the language and how to perform various tasks were only viable with it.

The idea of an opaque pointer is much more maintainable and understandable for an API that is to be used for either C or C++; or even just the one. It ensures type safety and is easy to read.

In the header file for the API you have:

// forward declare a struct called MyThing, but make sure the contents of it aren't available so that we can do anything we want to it in the implementation.
struct MyThing;

// define a class that contains function pointers.  We have a requirement that these function pointers must be valid for the lifetime of them being registered with MyThing.
struct MyThingCallback {
    void (*eventA)(MyThing* sender, const char* someData);
    void (*eventB)(MyThing* sender, int someOtherData);
};

// Some helper functions to do things with MyThing
MyThing* createMyThing();
void deleteMyThing(MyThing*);
void registerCallback(MyThing*, MyThingCallback);

int getSomeProperty(const MyThing*);
void setSomeProperty(MyThing*, int);

In the implementation (which includes this file) you then have

struct MyThing {
private:
    int property;
    std::vector<MyThingCallback> callbacks;
    // functions are allowed here
};

You will note that the API user doesn't need to know anything about MyThing because they never access any members of it; they only ever use a pointer to it. The idea of a void*, or even different objects being passed for different events has now been removed and everyone can always be sure that their function will only be used at the right time.