On Fri, Dec 2, 2016 at 9:19 AM, Edward Diener
For your review you may wish to consider the following questions:
- What is your evaluation of the design?
The design looks reasonable.
- What is your evaluation of the implementation?
I found the use of typedef's to functions returning inline struct definitions to be overly clever and cryptic to new C++ developers. I also found the lack of static type checking in certain places to be objectionable. The unusual formatting is an important subjective turnoff to potential users. Presumably the goal of the library is to enable users to get the functionality they've been missing. The hurdle of a new formatting style to get used to goes against this goal. Fortunately, this is something easily remedied.
- What is your evaluation of the documentation?
Looked reasonable.
- What is your evaluation of the potential usefulness of the library?
Given the prior existence of Boost.Signals I wouldn't find this library to be useful. There is an important aspect to this review that I want to call out. A signal is a "vocabulary type" in that it is pervasive through systems and frequently used in public interfaces. One of the most important qualities of a vocabulary type is that there be only one of them. Imagine the confusion if we had one thread-safe shared pointer class and another one optimized for single threaded usage, or if we had an suite of different variant types with different tradeoffs. No, there should only be one signal type and Boost.Signal made the correct choice of using a so-called intrusive design. - Did you try to use the library?
Nope.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I spent a few hours on the review. A small amount of time was reading the documentation and a much longer amount of time was spent pondering the implications of incorporating the library into Boost. - Are you knowledgeable about the problem domain?
Yes. I've used Qt signals, Boost.Signals1, and Boost.Signals2 extensively in various projects.
- Do you think the library should be accepted as a Boost library?
No. -- David Sankel