Re: [boost] Review: Regular Expression Library

From: Jens Maurer (Jens.Maurer_at_[hidden])
Date: 2000-08-23 13:08:02

John Maddock wrote:
> > - regbase::flag_type is a bitmask type, but it does not define
> >the appropriate operators | & etc.  regbase::flags() returns the
> >wrong type.  For example, this code fails:
> >
> >        using boost::regbase;
> >        regbase::flag_type f = regbase::use_except | regbase::newline_alt;
> >
> >Look at iostream's flags how to do it correctly.
> 
> Yep, I guess the alternative is to use "static const unsigned = value;"
> syntax, but "that compiler" doesn't support that.

The alternative I had in mind is to implement the | & ^ bitwise operators
for regbase::flag_type.

> > - regex_split uses a container for output, but I think an OutputIterator
> >would be sufficient and more flexible.
> 
> I take your point, but I've found a container much easier to use in
> practice, [...]

Is it really so inconvenient to use "back_inserter(c)" instead
of "c" in the caller?

> > - regex.cpp:  bad_pattern::what() uses a local typedef for the base
> >type, but bad_expression::what() does it directly.  This is inconsistent.
> >I suggest removing the typedef from bad_pattern::what().
> 
> Nope, that's a VC6 workaround - cl can't cope with std::x::y::foo() syntax.
> 
> > - regex.cpp:  bad_pattern, bad_expression: Why do you implement the
> >what() member function at all?  This will be delegated to the base
> >class what() anyway.  If you need to implement some function so that
> >the virtual table shows up in the DLL, use an empty destructor.
> 
> Yes that's the reason, does it matter which function is paced here?
> Whatever I'll change to the destructor if you prefer...

This is a side issue, but let me elaborate a litte.  As someone else
said repeatedly, programming is a lot about communication. When I
see the bad_pattern and bad_expression class definitions in the header,
I start wondering "hm... he re-implements the what() member functions.
He must do some special things in them. Strange."  When I look at the
implementation, I notice that nothing special is going on.

And, as mentioned above, while implementing bad_pattern::what(),
you hit compiler breakage.  Wouldn't it be so much easier with a
destructor?  The destructor is virtual as well, so your DLL issues
should be solved.  You don't need (and are not allowed) to call the
destructor of the base class explicitly.  Regarding the communication
aspect, I would interpret much less in a re-implemented destructor
compared to a re-implemented public member function.  Additionally,
the destructors are empty functions { }, so it's very very easy to
see that nothing special is going on.

[sub_match, conversions to int etc.]
> Yes, I think I'll take those out, trouble is, they're *extremely*
> convenient at times, I wish their was some way to overload static_cast so
> that explicit casts would be allowed but implicit casts not found.

We had some stream-based one-function-call string-to-T converter nearly
ready for submission a few weeks ago ("stream_convert" or somesuch).
What has happened to it, btw.?

>    Would
> you be happy with a "int toi()" member function or is that tempting fate
> <bg>?

I'm a bit less miserable with toi() instead of the implicit casts, but
I'm definitely not happy :-(

Jens Maurer