Pass event callbacks by reference

[fhd]

Background

We're currently passing std::function objects by value in JsEngine::SetEventCallback and FilterEngine::SetFilterChangeCallback. Because of this, using stateful functors is not possible.

What to change

Pass event callbacks by reference in JsEngine::SetEventCallback, FilterEngine::SetFilterChangeCallback and FilterEngine::SetUpdateAvailableCallback. Event callbacks are currently being stored by value in an std::map in JsEngine - we'll have to store a pointer instead.

Once this is done, we should check client and test code that works with event callbacks - we will be able to get rid of some hacks there.

[fhd]

Adding Sergei and Wladimir.

What do you guys think? IMO this would be more intuitive and technically possible.

[sergz]

I'm sharing by subjective opinion.

I would say that while we operate with std::function the statefulness should be implemented on the client side and the client should be ready that the delegate can be copied by the library several times. Such restriction is common in STL and it's more error-tolerant (one may make a mistake but it still does work).

But passing by the const ref would be definitely a good optimization, so I would change only this part.

If it somehow does not fit the client circumstances (for example, it may be difficult to store the pointer in closure and properly release it, who knows) I would replace std::function by interface with a single method, in this case we will not make a copy of instance by accident and the client is aware about that by API. Also the lifetime of the passed object should be clear for the client

• FilterChangeCallback*/reference_wrapper<FilterChangeCallback> - not managed by the library, should be disposed by the client after destroying the filter instance
• unique_ptr/auto_ptr<FilterChangeCallback> - the library is responsible for the disposing of the delegate
• shared_ptr<FilterChangeCallback> - the delegate is available for the client as well as for the library and its lifetimes depends on the usage counter

[sergz]

We are passing callbacks by const ref everywhere and the client should be ready that the function object will be copied. Therefore, I'm closing this issue.