Subject: Re: [boost] [Boost-users] Subject: Formal Review of Proposed Boost.Process library starts tomorrow
From: Boris Schaeling (boris_at_[hidden])
Date: 2011-02-09 18:13:07
On Tue, 08 Feb 2011 08:50:59 +0100, Denis Shevchenko
<for.dshevchenko_at_[hidden]> wrote:
> [...] Macros will be easier for maintenance if align backlslashes.
> Compare:
>
> #define BOOST_PROCESS_THROW_LAST_SYSTEM_ERROR(what) \
> boost::throw_exception(boost::system::system_error( \
> boost::system::error_code(BOOST_PROCESS_LAST_ERROR, \
> boost::system::get_system_category()), \
> BOOST_PROCESS_SOURCE_LOCATION what))
>
> and
>
> #define BOOST_PROCESS_THROW_LAST_SYSTEM_ERROR(what) \
> boost::throw_exception(boost::system::system_error(
> \
> boost::system::error_code(BOOST_PROCESS_LAST_ERROR, \
> boost::system::get_system_category()),
> \
> BOOST_PROCESS_SOURCE_LOCATION what))
Are you saying that the second version is better? I wonder as I see the
first version in the code?
> 3. In constructor of class boost::process::context.
>
> In this place you insert a values into map, but not search them.
> So use insert() instead of operator[]. Of course, with
Ok.
> 4. In some classes you use public inheritance from boost::noncopyable,
> but you can use private inheritance instead.
True, will also be changed.
> 5. In constructors of some classes you forgot 'explicit'.
Where do you think we should add 'explicit'?
> [...]11. In many places of code you using std::map. May be better to use
> boost::unordered_map, for optimizing the speed of searching?
Again true. I think in most cases ordering doesn't matter.
I've added everything to my list of proposed changes (trying to keep track
of everything :). I'll send the complete list at the end of the review as
there will be probably some more ideas until then.
Thanks,
Boris