Re: Thoughts on Tokenizer

From: jbandela_at_[hidden]
Date: 2001-06-03 19:43:10

Responses are below comments

> 2.  My compiler couldn't match the container & tokenizer-functor 
constructor
> of "tokenizer" with the first three tokenizer tests.  If I 
commented out the
> "const" from the declarations of "test_string," that code passed.  
I guess
> the qualification screws up the type matching.

I think that is due to a mistake on my part. The constructors that 
take Container, should take a const reference rather than just the 
reference they do now. Thanks for pointing it out.
 
> 3.  Even though all instances of "offset_separator" have constructor
> arguments, the constructors of "tokenizer" and "token_iterator" 
insist that
> the _possibility_ of default constructing the tokenizer-functor 
type must
> exist.  Since "offset_separator" has no default constructor, my 
compiler
> chokes on it.  (The errors are highlighted in "tokenizer.hpp".)
> 
offset_separator does not have a default constructor, because I am 
not sure of what the semantics of a default constructed 
offset_separator should be.

> *   If detail::tokenizer_base is expected to be the base class for 
a "Base"
> class that is a template parameter for templated member functions of
> tokenizer_policy, then tokenizer_base should move to the 
outer "boost"
> namespace, because it is a published component.  If not, then don't 
publish
> it in the documentation and change the other items in the file to 
not
> require it.  The token_iterator_generator and make_token_iterator 
templates
> directly depend on the existence of tokenizer_base.  The 
tokenizer_policy
> class template practically depends on tokenizer_base, because the 
class
> template uses the exact syntax of tokenizer_base.  (Run-time 
information
> from one class to another should be a member function, to allow the
> possibility of on-the-fly generation of the information.)
> 

tokenizer_base is unpublished because the user should never have to 
use it. Based on other comments, tokenizer_policy should also  be 
unpublished.

> *   A generator, in STL lingo, is a function object that takes no 
arguments
> and returns a value, so "token_iterator_generator" has an 
inappropriate
> name.  Also, a traits class that just provides a type should only 
have an
> interface that consists just of that type (we'll call it "type").  
You mix
> metaphors by having a main "type" and other published types.  That 
looks
> confusing.  Either rename "type" to something else or unpublish the 
helper
> types.

The name generator is used because iterator_adapter uses it. The 
other typedefs should be private.

> *   I don't like the interface that involves the first iterator 
argument
> being a reference.  Other libraries and STL almost always take 
iterator
> arguments as values.  The std::codecvt facets use your idea a 
little bit.
> An alternative would be to return a std::pair<Iterator, bool>, 
where the
> second part is what you already return and the first part is the 
new next
> iterator.

I do not see why the reference is so bad, since these functions will 
only be called from tokenizer_policy and not by the user.
 
> *   The csv_separator and punct_space_separator could take another 
template
> argument, to define a traits class for comparison, like strings and 
I/O do.
> The default value would be std::char_traits<Char>.

Good idea.

> *   For csv_separator, shouldn't the separating character go first 
in the
> constructor?  I think that would be what changes the most.
> 

I personally have changed the escape character more. For example, I 
have changed it before to ^ since in files that include pathnames, 
since it is a pain to escape out each \ in the pathname in Windows.

Thanks for your comments,

John R. Bandela