
Hi Everyone, The library review starts still in two weeks, but I wanted to give some early feedback. First, I wanted to thank Jean-Louis for exploring the subject, writing and sharing the library, and for submitting it for the Boost Review. My feedback is based on the reading of the documentation, I haven't played with the library just yet. [1]. Animals as examples There are already a number of things in the library that suggest it might not be suitable for production use (I describe them below), but using class Animal as explanatory examples amplifies the impression that this is going to be a toy library. Half of the books I have read on OO use the Animal (or Employee) example, but never in my C++ career did I have to use OO to model something close to "animal". I know they are just examples, but picking one so detached from programming reality is in my estimation harmful. The arithmetic expression example is way better. I, for one, use OO for representing dynamically-configurable parts of the applications, such as plugins or user-selectable sorting or selection predicates. [2]. The motivation In C++ we already have a dynamic-dispatch tool for "closed set of methods with open set of types": virtual functions. We also have a dynamic-dispatch tool for "open set of methods with closed set of types": the visitation of std::variant. You need to provide a motivating example (more convincing than the zoo) that needs exactly this: "open set of methods with the open set of types". The arithmetic expression example doesn't do the job: there is a fixed and well known list of concrete types. Also, the fact that Bjarne Stroustrup wanted to add open methods long ago is insufficient a motivation. [3]. Are open methods compatible with static type checking? It looks that when one of the two things -- number of types, number of methods -- is known statically, the compiler can verify if the programmer didn't forget to take some type or method into account. (Well, sometimes we get the "pure virtual function call" error, but these are very rare cases.) The same seems not to be the case for open methods. What this library advertises as flexibility would be the source of many programmer bugs detected only at runtime, possibly by end users. Imagine that programmer Alice is adding her new type A and plugs it into open methods known to her, and at the same time programmer Bob in another file adds a new open method B known only to him. Bob doesn't know about class A, and Alice doesn't know about open method B. The ISO C++ proposals quoted in the docs mentioned solving this by a mandated or encouraged linker error. Is it possible for this library to diagnose the incomplete specification of an open method in the `initialize()` function? The docs use an expression, "when an error is encountered [...]". But note that in C++ we often use the term "error" to express the situation where a _correct_ program encounters a hostile situation in the environment (lack of resources, bad user input). But openmethod docs actually mean something different: a programmer bug. It is very easy for a programmer to plant a bug in this flexible design. [4] In a similar vein, the decision to select an arbitrary specialization of an open method upon a tie, is another runtime surprise: not even a reported error. For multi methods it is not even clear how a static check for exhaustive unambiguous specialization should work. [5] It is concerning that so many things are defined in the global namespace. The docs say, "First we need to include the library’s main header. It defines a few macros, and injects a name - virtual_ptr - in the global namespace." But then the reference part says that virtual_ptr is in the namespace boost::openmethod. Who is right? The fact that as common a name as `next` is reserved inside open methods is also uncomfortable. Why not use a namespace-scoped name? Next, the docs use the using-directive: `using namespace boost::openmethod;`. This is a bad idea: both for real programs and for the documentation: it is not clear if the library was designed with namespaces in mind. Also, all the examples define open methods in the global namespace. I instead would like the docs to show how to declare open methods in my namespace, as this is a far more probable use case. [6] Friendship It puzzles me how the notion of friendship can be combined with this idea of flexibility in open methods. It is my understanding that open methods are added on top of the existing classes. That is: classes do not have to know that some open method will be added in the future. In other words, open methods are non-intrusive, right? So how would a class know who to grant the friendship to? Also, the examples use the declaration `friend struct`, even though we are talking about methods. I think docs should mention the existence of these structs more prominently. [7] Function signatures Some macros in the library use function types in declarations: `BOOST_OPENMETHOD_OVERRIDERS(poke)<void(std::ostream&, virtual_ptr<Cat>)>` Others specify the type separately: `BOOST_OPENMETHOD_OVERRIDE(value, (virtual_ptr<Plus> node), int)` Is there a reason for this? Can the clever use of macros not offer the same function signature syntax? [8] Passing virtual pointers to const objects If `virtual_ptr<Cat>` is an analogue of this pointer, could we see an example in the docs that shows a virtual pointer to const objects? It would be `virtual_ptr<const Cat>`, I guess? [9] Is this library intended for production use? The library comes with a lot of clever macros, a lot of pointer type templates, a lot of new syntax and runtime surprises (mentioned earlier). I am concerned about how the debugging would look if something goes wrong with the usage of this library. Would you recommend it for use in production systems? Or is it for experimentation? [10] Minor things Section Custom RTTI has sentence:
Now we can include the "core" header
and then header `boost/openmethod/core.hpp` is not included. Is it a bug? Regards, &rzej;