Re: [boost] Tokenizer review comments

From: Douglas Gregor (gregod_at_[hidden])
Date: 2001-06-05 09:52:23

I still have a few comments on Tokenizer to throw in:

Code nits:
        - 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.

        Doug Gregor