Re: [boost] A reminder for dlw_oprs.zip

From: David Abrahams (abrahams_at_[hidden])
Date: 2000-11-05 11:30:05

Remarks

-------
I'd like you to consider different names for the new operator templates. All
of the old ones (aside from the nearly useless "operators" template) are
adjectives, e.g. "addable". I'd like to see that continued, e.g.:

    capable_of_not => logically_invertable
    comparable_operators => totally_ordered
    additive_operators => additive?
    multiplicative_operators => multiplicative?
    integer_multiplicative_operators => integer_multiplicative?
    combining_bitwise_operators1 => bitwise_combinable
    sequence_traveling_operators => bidirectionally_incrementable
    shifting_bitwise_operators2 => bidirectionally_shiftable or
bitwise_shiftable

I guess this may not apply to the archetype classes. Something to think
about.

------
Carefully looking over the code with ediff shows that this fragment has been
dropped. Was that intentional?

#ifndef BOOST_NO_TEMPLATE_PARTIAL_SPECIALIZATION
template <class T, class I, class R, class B>
struct is_chained_base< ::boost::indexable<T, I, R, B> > {
  typedef ::boost::detail::true_t operator_template_type;
};
#endif

-------

I'd want Jeremy to look over the stuff related to iterators

-------

iterators_test.cpp - Mostly, I'll leave evaluation to Jeremy, but:

once again, some arbitrary reformatting makes the evaluation harder.
Especially annoying is the insertion of whitespace in "operator--", inside
parentheses, etc. It's fine if you want to write new code this way, but it
just makes my job harder. Moreover it is impolite to reformat other peoples'
work to suit your own stylistic preferences.

test_iter has been changed to take a new P type parameter, and stores one
internally, but still takes a T* value parameter to its constructor. Is this
intentional?

names like TITRP are cryptic and hard-to-read.
ALL_CAPS names should be reserved for macros to help avoid name collisions.
TITRP is exactly the sort of name someone is likely to have used for a macro
thinking that nobody else would want to use it.

-------

documentation: Looks excellent overall

The headers in the tables are great. I'd like you to try breaking the
sentences onto separate lines or bold-ifying the symbol names. It's a bit
hard to read on my browser because the tiny periods don't break up the words
sufficiently, especially when some of the words are as short as "T" and "U".
I prefer separate lines, as that mirros the standard text.

One other nitpick: there's something a bit clumsy about the description of T
and U in the table, and the description of U in particular is fuzzy. That's
why I left the descriptions out and "let the table do the talking".  I think
it's completely obvious from the operations column what role T and U play.

Hmm. I don't like your new column label "operations". It used to say
"template will supply". Now it isn't clear that we are providing the
operations listed. Details like this are part of the reason I would prefer
minimal reformatting of the html. I don't know how many others might be
slipping past my notice.

When I encountered this text:
"Many types that define an operation commonly include related operations.
The grouped operator templates use base class chaining, and can be used with
the technique (via an unshown parameter), to provide pairs or triplets of
the simple operator template classes, or other grouped operator template
classes, at once."

I thought it went with the previous table. Maybe a horizontal rule or
something to set it off from the previous table would help. Also, the word
"unshown" is clumsy. How about "hidden"? Also clumsy: the use of "many" and
"commonly" in the same sentence. The mention of base class chaining IMO is
incidental and only for completeness. I'd prefer this text:

"The grouped operator templates provide common groups of related operations.
For example, since a type which is addable is usually also subractable, the
additive template provides the combined operators of both. The grouped
operators have an additional optional template parameter B, which is not
shown, for the base class chaining technique."

There's a lot of vertical whitespace in the 2nd column of the  grouped
operators table. Is that neccessary? It makes it hard to take in at a glance
IMO. I favor leaving out the following text in each cell and moving
commonality to the column header:
"Supports the operations and needs the requirements of the following <T>
templates: "

I believe you have misspelled "archetype" as "archtype" repeatedly.

I would like to see cross-linking in the tables. If I look at the table for
operators<T>, it mentions 4 other templates in terms of which it is defined.
I want to be able to follow the links to find out what those templates are.

Don't forget to update the revision date.