cajun-rat
commented
2 years ago What's the goal, though? I understand your advantages, but I don't see that you're actually taking advantage of them, such as by adding extra metadata.
Good question. I personally find 'functional compositional' approaches (std::bind + std::packaged_task) harder to reason about than approaches based on objects and inheritance. Annoyingly (for my implementation) a specialization of CommandBase is needed to handle the void case, which is automatically handled by the std::bind + std::packaged_task approach.
If your view is 'yes this is much nicer, I've always found that thing confusing', then I suggest merging. If you're not convinced this is easier to read, then I'll happily drop this until there is a stronger reason to include it. The first two commits of the series are probably valuable, I'll tidy those up into a separate PR.
As part of work on offline updates, I pulled the API apart and rewrote it in a more explicit style. It turns out that I could make the offline changes independently of this, so I've split this change out.
The first two commits are pretty standard, but the last one is potentially controversial. I'm interested in your opinion on the relative readability of the before/after versions.