launchdarkly / c-client-sdk

LaunchDarkly Client-side SDK for C/C++
Other
7 stars 15 forks source link

Allow passing a "closure" argument to the feature flag listeners #84

Closed ehsan closed 1 year ago

ehsan commented 2 years ago

Is your feature request related to a problem? Please describe. It's common practice for C APIs which allow registering a callback to allow the consumer to pass a "closure" in the form of a void* argument which will be passed through directly to the callback once it is called. This parameter sometimes has other names, like "user data", "context", etc.

It would be great if registerFeatureFlagListener() supported this as well.

Let me elaborate the use case with an example. Let's say that we want to run a main thread callback in response to a change in a feature flag.

void registerListener(MainThreadCallback callback) {
  auto* client = LDClientCPP::Get();
  client->registerFeatureFlagListener("foo", [](const char* const flagKey, const int status) {
    // We're being called on a LaunchDarkly thread, let's redirect back to the main thread.
    // Oops, now I have no way of accessing the `callback` variable from here. :-(
  });
}

In our fork of the SDK we added a new API to allow passing in a closure argument, which allows us to solve the problem like this:

void registerListener(MainThreadCallback callback) {
  auto* client = LDClientCPP::Get();
  client->registerFeatureFlagListenerWithClosure("foo",
    [](const char* const flagKey, const int status, void* closure) {
      // We're being called on a LaunchDarkly thread, let's redirect back to the main thread.
      MainThreadCallback mainThreadCallback = (MainThreadCallback)closure;
      DispatchToMainThread(mainThreadCallback);
  }, (void*)callback);
}

Describe the solution you'd like Either a separate API similar to the sketch above, or maybe modifying registerFeatureFlagListener() would work great for us, depending on whether backwards compatibility is a concern.

Describe alternatives you've considered If the listener is a global one, perhaps one could store the closure pointer in a global variable to access from within the listener function, but that wasn't a great option for us.

Additional context I'd be happy to submit a PR for this if you would kindly let me know which approach you prefer.

Thanks!

cwaldren-ld commented 2 years ago

Hi @ehsan , I have good news for you.

While I don't have a specific release date, we do have this feature on our radar, and additionally for LDSetClientStatusCallback which has the same limitation.

I would be interested in seeing your PR if it's not inconvenient. We must remain backwards compatible, so it would need to be a new function e.g. LDClientRegisterFeatureFlagListenerUserData (a mouthful, maybe we can come up with something shorter!)

If you could target it against the contrib branch that would be great.

Filed internally as 161115.

ehsan commented 2 years ago

Great to hear that and sounds good! Will do.

ehsan commented 2 years ago

@cwaldren-ld Submitted a PR: #85. Looking forward to hear your feedback. Have a great weekend!

kinyoklion commented 1 year ago

Hello @ehsan,

We have released a new C++ SDK, with C bindings, which supports using either a C++ closure, or a C function which accepts a user data parameter.

Thank you, Ryan