From: Douglas Gregor (gregod_at_[hidden])
Date: 2001-06-05 09:52:23
I still have a few comments on Tokenizer to throw in:
- the naming of variables and functions seems to be inconsistent. There are
names that follow the boost_convention, but others that alternate capitals or
have no separation between words.
- The include guards contain "JRB015801." Is this intended to be updated as
the tokenizer version is updated? I'm curious because it's not a practice
I've seen before.
The name "csv_separator" should probably be changed to something more obvious
("csv" is not a common acronym. Perhaps "comma_separator"?)
punct_space_separator::operator() has a common with some incorrect grammar:
"skip past the punctuation only if the told to do so"
The whitespace_and_punct class shouldn't be in the "detail" namespace if the
user is expected to specialize it for other character types.
Also, the names "punctuation1" and "punctuation2" don't convey much meaning,
though I'm at a loss to come up with better names. If whitespace_and_punct is
to be an interface element, I believe that it should only have "punctuation"
and "whitespace" functions. Perhaps the contents of "punctuation1" and
"punctuation2" should be separated into constants instead?
Perhaps the name "whitespace" in punct_space_separator is too restrictive.
strtok() calls these characters "delimiters," which seems to be more general.
Overall, I think that Tokenizer should be accepted into Boost.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk