trjhtr

I'd appreciate advice regarding both the implementation detail and ways to improve usage/performance/generic.

There are two parts: XDelegate, which is I got from here, and XEvent, which is the event implementation similar to C#.

#pragma once
#include <unordered_map>
//hashing function
//TODO: replace hashing with something else if it's too expensive
template<typename return_type, typename... params>
class XDelegate;
namespace std 
 template<typename return_type, typename... params>
 struct hash<XDelegate<return_type, params ...>> 
 typedef return_type(*Type)(void* callee, params...);

 static size_t hash_combine(size_t seed, size_t v)
 
 return seed ^= v + 0x9e3779b9 + (seed << 6) + (seed >> 2);
 
 size_t operator()(const XDelegate<return_type, params ...> &pDelegate) const
 
 size_t a = std::hash<void*>(pDelegate.fpCallee);
 size_t b = std::hash<Type>(pDelegate.fpCallbackFunction);
 return hash_combine(a, b);
 
 ;
;

//nice generic variadic delegate from http://blog.coldflake.com/posts/C++-delegates-on-steroids/
template<typename return_type, typename... params>
class XDelegate

 friend struct std::hash<XDelegate>;

 typedef return_type(*Type)(void* callee, params...);
public:
 XDelegate(void* callee, Type function)
 : _fpCallee(callee)
 , _fpCallbackFunction(function) 
 template <class T, return_type(T::*TMethod)(params...)>
 static XDelegate from_function(T* callee)
 
 XDelegate d(callee, &methodCaller<T, TMethod>);
 return d;
 
 return_type operator()(params... xs) const
 
 return (*_fpCallbackFunction)(_fpCallee, xs...);
 
private:
 void* _fpCallee;
 Type _fpCallbackFunction;
 template <class T, return_type(T::*TMethod)(params...)>
 static return_type methodCaller(void* callee, params... xs)
 
 T* p = static_cast<T*>(callee);
 return (p->*TMethod)(xs...);
 
;


//C# style delegate-event for none ref class.
template<typename return_type, typename... params>
class XEvent

 friend struct std::hash<XDelegate<return_type, params ...>>;

public:
 size_t add(XDelegate<return_type, params ...> pDelegate)
 
 size_t token = std::hash<XDelegate<return_type, params ...>>(pDelegate);
 auto result = _delegates.insert_or_assign(token, pDelegate);
#ifdef _DEBUG
 //if assignment took place, ie) there is a hash collision, throw exception.
 if (!result.second)
 
 throw std::exception("XEvent hash collision");
 
#endif
 return token;
 
 void remove(size_t pToken)
 
 _delegates.erase(pToken);
 
 size_t size() const
 
 return _delegates.size();
 
 //on hindsight, I don't think C# events have return value?
 std::unordered_map<size_t, return_type> operator()(params... xs)const
 
 std::unordered_map<size_t, return_type> result;
 for (auto itr = _delegates.begin(); itr != _delegates.end(); ++itr)
 
 result[itr->first] = itr->second(xs...);

 
 //return map of event tokens mapped to the results.
 return result;
 
private:


 std::unordered_map<size_t, XDelegate<return_type, params ...>> _delegates;
;


template<typename... params>
class XEvent<void, params ...>

 friend struct std::hash<XDelegate<void, params ...>>;

public:
 size_t add(XDelegate<void, params ...> pDelegate)
 
 size_t token = std::hash<XDelegate<void, params ...>>(pDelegate);
 auto result = _delegates.insert_or_assign(token, pDelegate);
#ifdef _DEBUG
 //if assignment took place, ie) there is a hash collision, throw exception.
 if (!result.second)
 
 throw std::exception("XEvent hash collision");
 
#endif
 return token;
 
 void remove(size_t pToken)
 
 _delegates.erase(pToken);
 
 size_t size() const
 
 return _delegates.size();
 
 void operator()(params... xs)const
 
 for (auto itr = _delegates.begin(); itr != _delegates.end(); ++itr)
 
 itr->second(xs...);
 
 
private:
 std::unordered_map<size_t, XDelegate<void, params ...>> _delegates;
;

It is used like this:

_returnEventTokens.push_back(_returnEvent.add(XDelegate<int, std::wstring>::from_function<EventTester, &EventTester::returnEventCallback1>(_tester)));
_returnEventTokens.push_back(_returnEvent.add(XDelegate<int, std::wstring>::from_function<EventTester, &EventTester::returnEventCallback2>(_tester)));
_returnEventTokens.push_back(_returnEvent.add(XDelegate<int, std::wstring>::from_function<EventTester, &EventTester::returnEventCallback3>(_tester)));


_returnEvent(L"Return event called!n");

for (auto itr = _noReturnEventTokens.begin(); itr != _noReturnEventTokens.end(); ++itr)

 _noReturnEvent.remove(*itr);

Your description of "it is used like this" leaves quite a bit to the imagination. For example, what are the types of _noReturnEventTokens, _returnEventTokens, and _returnEvent? How are they declared? What do you expect to happen when you evaluate _returnEvent(L"Return event called!n")?

I don't think the code is so "unpleasant" to read as CodeGorilla suggested; but it could definitely be improved by some minor reformatting. I strongly suggest putting a blank line between each pair of member function definitions: that is, not

auto f()


auto g()

but instead

auto f()



auto g()

. This is minor but would help quite a bit in "breaking up" the code visually. If you want to get the vertical real-estate back, then cuddle your braces:

auto f() 


auto g() 

void operator()(params... xs)const

 for (auto itr = _delegates.begin(); itr != _delegates.end(); ++itr)
 
 itr->second(xs...);
 

The first thing to notice about this code is that it contains an old-style (non-range) for loop. Rewrite it as

 for (auto&& pair : _delegates) 
 pair.second(xs...);
 

The second thing that caught my eye was the construct params... xs. Taking a parameter pack by value is rare; taking by perfect-forward (params&&... xs) is more common. But because you're doing a lot of metaprogramming here, maybe by-value is correct. So I kept looking to find out where params came from — and I think all your uses are reasonable. There was one place I thought you should have been using perfect-forwarding, but then I realized that you probably did need to avoid moving-out-of the arguments prematurely (because they would be used again by other delegates).

Incidentally, I recommend CamelCase for template parameters. What you called params..., I would have called Args... or possibly just Ts....

namespace std 
 template<typename return_type, typename... params>
 struct hash<XDelegate<return_type, params ...>> 
 typedef return_type(*Type)(void* callee, params...);

 static size_t hash_combine(size_t seed, size_t v)
 
 return seed ^= v + 0x9e3779b9 + (seed << 6) + (seed >> 2);
 
 size_t operator()(const XDelegate<return_type, params ...> &pDelegate) const
 
 size_t a = std::hash<void*>(pDelegate.fpCallee);
 size_t b = std::hash<Type>(pDelegate.fpCallbackFunction);
 return hash_combine(a, b);
 
 ;
;

That last }; should be just }, and I strongly recommend adding the comment // namespace std for readability. You'll see this "end-of-namespace comment" convention being used in most C++ codebases these days.

But for a simple specialization of std::hash, you actually don't need to reopen the namespace — you can just specialize the qualified struct std::hash, like this:

template<class R, class... Args>
struct std::hash<XDelegate<R, Args...>> 
 static size_t hash_combine(size_t seed, size_t v) 
 seed ^= v + 0x9e3779b9 + (seed << 6) + (seed >> 2);
 return seed;
 

 size_t operator()(const XDelegate<R, Args...>& pDelegate) const 
 using Type = R(void *, Args...);
 size_t a = std::hash<void*>(pDelegate.fpCallee);
 size_t b = std::hash<Type*>(pDelegate.fpCallbackFunction);
 return hash_combine(a, b);
 
;

Observe the cuddling of braces, the blank line between function definitions, the CamelCase template parameter names (and much shorter — return_type becomes R), and the modernized using T = U in place of typedef U T.

Observe that instead of declaring Type as an alias for a function-pointer type, I declared it as a function type and moved the * down into the place it was used, in std::hash<Type*>, which makes the two std::hash<X*> invocations nicely symmetrical.

Observe that I never put two side-effects on the same source line of code. Instead of return seed ^= expr;, I write seed ^= expr; and then on the next line return (the new value of) seed. This really matters when you start getting into atomic/lock-free stuff, but it never hurts.

friend struct std::hash<XDelegate>;

Consider giving XDelegate a non-static member function hash(), so that if you want the hash of an object x you just say x.hash(), instead of std::hash<decltype(x)>(x). You'd still have to provide a trivial out-of-line specialization

template<class R, class... Args>
struct std::hash<XDelegate<R, Args...>> 
 size_t operator()(const XDelegate<R, Args...>& x) const 
 return x.hash();
 
;

but it would no longer need to be a friend of the class.

I'm sure there's more event-and-delegate-relevant kinds of things to talk about, but this is where I'll stop. :)

The code is not very pleasant to read, its like it has been autogenerated. I wouldn't like to have to pick up from you and fix a bug in it. It may be a very clever way of doing whatever it is you are trying to do, but I don't think its good code.

It has very little in the way of meaningful comments, nothing to describe what the purpose of the templates and functions are. If you combine this with the links to websites in my experience it implies that the person writing it doesn't really understand what the code does.

Why do you only throw exceptions in DEBUG builds, does the code never go wrong in RELEASE builds?

One question I can't get out of my head is if you want C# delegates and events why not program in C#, otherwise just use what is already in C++?