On Tue, 08 Feb 2011 08:50:59 +0100, Denis Shevchenko <for.dshevchenko@gmail.com> 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