2
\$\begingroup\$

Recently, I found myself frequently writing notification managing facilities. That is, one can register as a receiver by providing a callback, and another can send a notification to all registrants by invoking their corresponding callbacks. I had been bothered by re-writing this similar code again and again. Finally, I decided to write a generic one once and for all. Following is the version for cases where the maximum number of registrants is known and limited. So I don't bother using advanced data structures for managing the registered callbacks. Comments pertaining to any aspect of the code are welcome :)

#include <array>
#include <functional>
#include <mutex>
#include <stdexcept>
#include <utility>

template <typename Sig, int max_reg = 16, typename Func = std::function<Sig>>
class NotifyRegistry;

template <int max_reg, typename Func, typename R, typename... Args>
class NotifyRegistry<R(Args...), max_reg, Func>
{
  static_assert(max_reg > 0, "");
public:
  template <typename F> 
  int Register(F&& f) {
    std::lock_guard<std::mutex> lock(mut_);
    int i = -1;
    while (++i < max_reg && reg_[i]);
    if (i == max_reg) throw std::length_error("");
    reg_[i] = std::forward<F>(f);
    return i;
  }
  void Unregister(int id) {
    if (id < 0 || id >= max_reg) throw std::invalid_argument("");
    std::lock_guard<std::mutex> lock(mut_);
    reg_[id] = nullptr;
  }
  void Notify(Args... args) const {
    std::lock_guard<std::mutex> lock(mut_);
    for (auto& f : reg_) {
      if (f) (void)f(std::forward<Args>(args)...);
    }
  }
private:
  std::array<Func, max_reg> reg_;
  mutable std::mutex mut_;
};
\$\endgroup\$
  • \$\begingroup\$ one question for this line if (f) (void)f(std::forward<Args>(args)...);. Why you need add a (void) here? \$\endgroup\$ – 2power10 Dec 13 '17 at 6:28
  • \$\begingroup\$ @2power10 To explicitly denote that the return value is ignored. \$\endgroup\$ – Lingxi Dec 14 '17 at 8:31
4
\$\begingroup\$

Your class seems generally well done and easy to use. I have only a few suggested improvements.

Add text to throws

It would be useful to have something other than a null string when the object throws an error.

Add text to the static_assert

The same issue exists for the static_assert in that it would be useful to provide a meaningful error message instead of an empty string.

Provide some comments

Because the class is compact and generally well written, not much explanation is required, but a brief explanation, especially noting which exceptions might be thrown, would be useful.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the suggestions. While providing text to the exception object may be useful, I don't think it's quite necessary to do so with the static_cast. max_reg > 0 is already descriptive. \$\endgroup\$ – Lingxi Apr 26 '15 at 15:23
  • \$\begingroup\$ It's your call, of course, but I think I'd add something like "NotifyRegistry requires a positive number of maximum registrations" to make it easier to understand why the error is thrown and from whence. ` \$\endgroup\$ – Edward Apr 26 '15 at 15:33

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.