Re: [boost] [Boost-users] Subject: Formal Review of Proposed Boost.Process library starts tomorrow

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