From: Aleksey Gurtovoy (alexy_at_[hidden])
Date: 2000-12-18 12:37:52
I also think that we're 'almost there', though there are a few issues I
would like to resolve before the new version goes public. My overall comment
is that both code and documentation look really good, good work Daryle!
BTW, comments are listed in the order I looked at things, not in the order
of their significance ;)
1) it might sound as nit-picking, but there is a difference between 'class
templates' and 'template classes' terms, and IMO the following (first)
sentence of the documentation should really use the former term ('class
templates'): "The header <boost/operators.hpp> supplies several sets of
template classes (in namespace boost)." BTW, some other boost libraries'
docs (e.g. one of type_traits), also use the 'template classes' phrase in
(IMO) wrong contexts...
2) "These templates define many global operators in terms of a minimal
number of fundamental operators." (the second sentence) The word 'global'
here makes me nervous ;). May be this one is better "These templates define
operators in namespace scope in terms of a minimal number of class'
3) as Dave already mentioned, characterizing a potential argument type of
(arithmetic) operator templates as a "numeric" (in particular, in the
'Arithmetic Operators' section) is incorrect, especially taking into account
the fact that comparison operators are currently classified as "arithmetic
operators" too, and there are a lot of applications for them outside of any
"numeric" stuff... hmm.. I am starting to think that these should be really
moved to a separate category (in docs). Dave?
4) "built-in numeric types" ('Arithmetic Operators' section, the second
sentence) should be "built-in arithmetic" or "standard arithmetic" (3.9.1).
5) a mistype error - "Composite operator templates >>simply<< list what
6) sentence "The requirements for the types used to instantiate the simple
operator templates are specified in terms of expressions which must be valid
and by the return type of the expression." is repeated two times - first
time at the end of the last paragraph of introduction text of the
'Arithmetic Operators' section, and second time at the beginning of the next
subsection - 'Simple Arithmetic Operators'. Probably we should just remove
the second appearance.
7) an important issue - template names; currently they are inconsistent in
sense that some of them do follow the standard's terminology and operators'
classification, and some of them don't or/and even use the standard naming
of operators categories to describe something different; for example,
'additive' and 'mutliplicative' match the standard terminology one-to-one,
and 'totaly_ordered' IMO doesn't even come close. The list of the names I'm
concerned about with some comments:
* 'totaly_ordered' - isn't plain 'comparable' a better choice?
* 'bitwise_combinable' - IMO should be called 'bitwise', and the current
'bitwise' should be renamed to something else or removed, because it
doesn't match the standard definition (5.11-5.13); IMO having a misguiding
name is much worse than a hypothetical inconvenience (two lines of code
instead of one) in rare cases.
* 'bidirectionally_shiftable' - again, just plain 'shiftable' (5.8)
should be good enough and more standard.
* 'logically_invertable' - operator ! is actually called 'logical
negation operator' (5.3.1 para 8), so it'll be more correct to call the
corresponding operator class template 'logically_negateable' or just plain
* 'unit_changeable' - Dave's 'bidirectional' make much more sense to me.
* 'idempotent' - well, there are no standard term for this (at least no
one that I know about :), and probably it is technically correct, but for me
it just seem to stand out against all others.
Also a question to all, highlighted by the recent discussion of the
'LessThanComparable' concept's requirements ;) - what about renaming the
'less_than_comparable' class template to something else, which will match
the standard definition more closely, and deprecating the old name??
'relational' (5.9 [expr.rel]) is not bad IMO.
8) HTML formatting issues:
* quote of Matt Austern in the 'Note for Users of Older Versions' is not
emphased in any way ("", or whatever) as such (at least not on IE5.5)
* a paragraph symbol (which is a part of the reference to wording of the
standard in the same note) is cute :), but the standard format of a
reference to the standard ;) is "Section 10.5 paragraph 5" and I think we
better stick to it.
One last meta comment: I am also sharing Dave's concerns about library's
grow (given that just recently I citied the library as an example of
appropriate layering/splitting into parts :). I think that splitting it to
at least three parts (both code and documentation) - basic arithmetic
operators, iterator adapters and grouped operators, should be done before
the release. If nobody else, I'll volunteer :).
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk