AMDG On 05/23/2012 07:31 AM, Larry Evans wrote:
On 05/22/12 15:34, Steven Watanabe wrote: [snip]
Online documentation can be found here: http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/
[snip] Looks very interesting.
I've done a somewhat brief review of just the online documentation. I'll try to do a more complete review later.
Here's my current comments:
* Virtual functions do not *require* dynamic memory:
The page:
http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/...
says:
C++ provides runtime polymorphism through virtual functions. ... They require dynamic memory management.
However, I don't think dynamic memory is required because just references to stack locations or pointers pointing to stack locations could be used to take advantage of virtual functions.
In theory, you're correct. It isn't strictly necessary. However, in practice, the cases where you can avoid new are the exception, rather than the rule. I don't want to complicate the introduction by being overly pedantic.
* 'Operation' term used before defined:
The page:
http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/...
says:
Operations can have more than one any argument.
before any definition of 'Operation' occurs. Only by seeing the:
"Operation" is not defined, because it doesn't have any special formal meaning in my library. Just take it in it's usual sense of something that you can do with an any.
addable<>
argument to the mpl::vector does the term 'Operation' begin to make sense, and then the reader has to guess that all the mpl::vector arguments are operations, and that all the previous 'operations' were unary.
The very next sentence explicitly contrasts *binary* addition with increment. I don't see how I can make this more clear without a lot of formalism that would just be off-putting for most people.
However, I'm still not sure that's correct. It would help if some link to 'Operation' defintion were provided everywhere that the term, 'Operation', occurs.
Maybe, where the term, 'opeation" occurs, there should be a link to:
http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/...
* 'match' term used before defined:
The page:
http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/...
says:
The types of the arguments must match or the behavior is undefined.
before any definition of 'match' or 'arguments' occurs. I'm just guessing from the arguments to x and y, that 'arguments' means arguments to the any CTOR, but then the CTOR argument to z is an expression; hence, I'm guessing that that expression is allowed by the addable<> template argument to the mpl::vector arg to the any_type.
arguments in this case means the arguments of +. types means the types that the anys hold. match just means that they're consistent with what addable<> expects i.e. the same type. This is the *tutorial*. I don't want to put all the technical details here. If you come away from this sentence with the idea that adding an any that stores an int to an any that stores a string is bad, it's clear enough for my purposes.
* public construct/copy/destruct item 1 undefined T:
The page:
http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/...
under:
1. template<typename U> explicit any(const U & data);
says:
Concept must not refer to any placeholder besides T.
However, T is not defined (or the definition is not obvious).
T is a class template parameter. I'm not sure what I can do to make it more obvious. Anyway, this comment is no longer correct.
* Mistitled Assign operator description: * Misplaced copy CTOR description:
This is all auto-generated by doxygen/boostbook. I'll see if I can figure out what went wrong.
* Typo "more general" in 'Design Notes.Constructors':
The page:
http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/...
says:
The last kind of constructor "upcasts" from one concept to another more general more general concept.
with the typo of duplicated "more general".
Fixed. In Christ, Steven Watanabe