Please see comments inline:
> Thorsten,
>
> Thanks for the reply. See comments
inline.
>
>> The change has happened long ago. It was a design
mistake to try to make
>> a range of singular iterators valid since it
adds overhead also for
>> those that don't need it.
>
>
When you say "overhead", are you speaking of size or speed?
> Was
this overhead demonstrated by some particular use case?
> Does this
mean you may be also removing the "singular" flag at some point?
> Is
there a supported way of constructing valid empty ranges? I can't see
that
> there's a way to do it.
I'm also not sure about the overhead issue here. Checking a single
bool inside size and/or empty strikes me as a trivial matter - no more complex
than many things in the standard library or elsewhere in boost.
<snip>
>
> IMHO, this is obfuscated. There is a difference between
an empty range and a
> range which is optional. In line with the std
containers, I would argue that a
> default constructed range should be
empty.
I have to agree with this premise. In line with a lot of proxy or
container type objects I would expect a default constructed object to do
something sensible. I wouldn't expect, for example, a boost.shared_ptr to
default construct to an object that asserts when you check it for NULL.
This change appears to be quite a severe change in functionality.
<snip>
> In any case, I've enjoyed using your library. I found it so
useful I've built
> four other libraries, which are mostly based on
passing data around through
> Boost.Ranges. I just wish that all of
my code wasn't broken now that
> I've upgraded.
>
This is a point on which I feel I have to amplify. I've been using
Boost for quite a long time now, and been through numerous upgrades, many of
which I've managed the upgrade at work. Breaking changes are always the
thing that hurts the most during an upgrade. Especially now that boost has
gone to an accelerated release cycle where we see such regular new
releases. I'm sure that many users of Boost like myself don't get to
upgrade to every release, so moving to a new release involves jumping through 2
or 3 releases. Breaking changes in this context are very painful. As
a library writer myself, where I am forced to do a breaking change, I always try
and make it something that will break on compile - otherwise my users cannot be
confident in doing a library upgrade without a very extensive testing schedule -
quite often this results in users simply not upgrading.
With this particular change, I feel even more strongly. Assertions in
library code can often be a bad thing. If the users of the library are not
explicitly aware of a change that takes out a check and replaces it with an
assertion, then you get applications which can either abort
unexpectedly (if not built with NDEBUG) or worse still just do something
different in a silent fail. This is just downright
dangerous. Personally, I would strongly support backing out of a change
like this. However, if Thorsten strongly disagrees, I would at least
recommend as a compromise position to introduce a define along the lines of
BOOST_NO_ASSERT (or maybe BOOST_RANGE_NO_ASSERT) that gets rid of the assertion
and does something sensible instead. The only times I like to see
assertions in libraries are: firstly, states which should be impossible to get
into and are therefore testing for either corruption or a problem in the
library; or secondly, occasions where you are going to crash anyway if you
continue - for example if after the assertion you are about to dereference a
NULL or uninitialised pointer. Things which are common states that systems
using the library could get into should do something sensible - at worst throw
an exception, and at best return a sensible answer.
Please reconsider this change - I have yet to upgrade to 1.33.7 yet, and
knowing this type of change is present there makes me highly reluctant to go
ahead with an upgrade. I'm not sure (just covering the post from Steven)
that going through an entire codebase and changing calls to empty with calls to
a new template function "is_empty_or_singular" is really a palatable
solution. Boost provides a fantastically useful set of libraries - they
have certainly significantly changed the way I code in C++; but breaking changes
should always be very carefully thought about, they have a nasty habit of
leaving people isolated in old versions of boost, and also have a habit of
driving people away from using boost.
Regards
Dave