[Boost-users] [optional] comments about none_t

From: Yuval Ronen (ronen_yuval_at_[hidden])
Date: 2005-02-21 17:13:15

Hi.

I checked out the optional library and found it to be of great value. I 
have, however, a few small suggested improvements (and no, I'm not 
talking about the pointer/container/discriminated-union semantics 
debate; I read this discussion in the mail archives and I also saw the 
resulted library and I'm very happy with what came of it).

I'm talking about the detail::none_t part.

Should it really be in the detail namespace? Isn't that a little weird? 
The detail namespace should be "forbidden area" for the users, yet here 
they are offered to write code like

   my_optional = boost::detail::none_t();

One might argue that there's the 'none' global variable which will make 
the code look like

   my_optional = boost::none;

which looks much better. I agree that it looks much better, but it still 
doesn't make the previous way of thinking (crating a temporary none_t 
instance) illegal or unsupported. It is unsupported now.

And while we're at the global none variable, I have to say that the 
reason I first tried the temporary-none_t-instance way, is that I didn't 
know of 'none'. The documentation is very poor on this subject. It's 
only mentioned in one place in a short, unexplained sentence, and it's 
in the "Detailed Semantics" section which no one ever bothers to read 
anyway.

In addition to improving the docs, I'd say that it's even better to 
include the boost/none.hpp header with optional.hpp so it could be 
easily found and reached. As I understood, the reason why this is not 
done now, is because some compilers (such as Borland's) have troubles 
with initialized data and pre-compiled headers. If this is truely the 
case, then I think it's easily solved. Include as follows:

#ifndef BOOST_OPTIONAL_DISCLUDE_NONE
#include <boost/none.hpp>
#endif

while the BOOST_OPTIONAL_DISCLUDE_NONE flag can be set either by the 
user, or automatically if a troublesome compiler is detected. If 
Boost.Config could automatically detect and set a 
BOOST_NO_INITIALIZED_DATA flag, then the optional library is left with 
fairly simple code.

To summarize, I suggest:
1. Lift none_t from detail to boost.
2. Improve docs wrt 'none' global instance.
3. Include none.hpp with optional unless the user/compiler dislikes it.

Makes any sense?
Yuval