Boost logo

Boost :

Subject: Re: [boost] [Boost-users] Subject: Formal Review of Proposed Boost.Process library starts tomorrow
From: Ilya Sokolov (ilyasokol_at_[hidden])
Date: 2011-02-08 03:37:39


On Tue, 08 Feb 2011 12:50:59 +0500, Denis Shevchenko
<for.dshevchenko_at_[hidden]> wrote:

> [snip]
>
> 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[].

+1

> Of course, with boost::assign ;-)

-1. No need to add a dependency in such simple case

> [snip]
>
> 6. In function boost::process::self::get_environment().
>
> environment e;
> // ...
> try
> {
> char *env = ms_environ;
> while (*env)
> {
> std::string s = env;
> std::string::size_type pos = s.find('=');
> e.insert(environment::value_type(s.substr(0, pos),
> s.substr(pos + 1)));
> env += s.size() + 1;
> }
> }
> catch (...)
> {
> // ...
> }
>
> And what kind of exceptions can occur here?
> std::string::find,
> std::string::substr,
> std::map::insert,
> std::string::operator+=
> std::string::size
> these functions do not throw exceptions...

???

> [snip]
>
> 8. In some places of code you using std::algorithms. You can use
> boost::range::algorithms instead, for simplicity.
>
> Compare:
>
> std::find(impls_.begin(), impls_.end(), impl);
>
> and:
>
> boost::range::find( impls_, impl );
>
> 9. You can replace some 'native' cycles by BOOST_FOREACH.
>
> 10. You can initialize map easier:
>
> [snip]
>
> 11. In many places of code you using std::map. May be better to use
> boost::unordered_map, for optimizing the speed of searching?

see above


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk