best practices using STL <algorithm>

[google]

Hi All,

I'm just getting started learning to use <algorithm> instead of loads
of little for loops, and I'm looking for a bit of advice/mentoring re:
implementing the following...

I have a vector of boost::shared_ptr's, some of which may be NULL. I'd
like to find out if there are any non-NULL elements in the vector.
Here's what I have at the moment.


typedef std::vector< boost::shared_ptr<::CFiCamera> > tCameraVector;
tCameraVector m_vpCameras;
....
....
bool CIIDCCaptureFacade::IsHardwareAvailable() const
{
//check for at least one initialized (non-NULL) camera
tCameraVector::const_iterator itCam = std::find_if(
m_vpCameras.begin(), m_vpCameras.end(),
std::bind2nd( std::not_equal_to<tCameraVector::value_type>(),
tCameraVector::value_type() )
);

return itCam != m_vpCameras.end();
}


Firstly, will this implementation meet the spec? I think it will.

Secondly, does anyone have any feedback on whether this is the 'right'
way to implement this using stl? Of course that's subjective, but I'm
looking for a little reassurance that I'm not getting into bad habits
right off the bat.

Cheers and TIA,

Pete

 

[Luke Meyers]

I'm just getting started learning to use <algorithm> instead of loads
of little for loops,

Cheers to that.

I have a vector of boost::shared_ptr's, some of which may be NULL. I'd
like to find out if there are any non-NULL elements in the vector.
Here's what I have at the moment.

typedef std::vector< boost::shared_ptr<::CFiCamera> > tCameraVector;
tCameraVector m_vpCameras;

bool CIIDCCaptureFacade::IsHardwareAvailable() const
{
//check for at least one initialized (non-NULL) camera
tCameraVector::const_iterator itCam = std::find_if(
m_vpCameras.begin(), m_vpCameras.end(),
std::bind2nd( std::not_equal_to<tCameraVector::value_type>(),
tCameraVector::value_type() )
);

return itCam != m_vpCameras.end();
}

Hmmm, well it's not exactly self-documenting as written, is it? One of
the reasons to use <algorithm> is to make your intent more transparent.
Since you're already in boostville, I'd suggest using the Boost Lambda
and/or Boost Bind libraries for your predicate. The STL binders and
function objects are really needlessly convoluted and verbose by
comparison.

Firstly, will this implementation meet the spec? I think it will.

AFAICT, yes. If it compiles and passes unit test, I'd say you're
probably safe (not on that basis alone, but based on that and the fact
that I don't see anything obviously nonstandard).

Secondly, does anyone have any feedback on whether this is the 'right'
way to implement this using stl? Of course that's subjective, but I'm
looking for a little reassurance that I'm not getting into bad habits
right off the bat.

If you really must do it without the mentioned Boost libraries, I'd
make it more clear by instantiating an actual (static const) null
object, outside of the loop, with an appropriate name, and use it for
comparison. However, you really don't need to do an equality
comparison -- what you want is to check for null. I believe shared_ptr
supports conversion to a boolean type based on nullness, so something
like the following should work:

template <typename T>
bool containsAnyNulls(std::vector< boost::shared_ptr<T> > const& vec)
{
using boost::lambda::_1;
return std::find_if(vec.begin(), vec.end(), !_1) != vec.end();
}

Caveat, I didn't take the time to test this code because you're just
asking about style. But it should work fine.

Actually, it might be a little more STL-style to do it like:

template <typename FwIter>
FwIter find_null(FwIter begin, FwIter end)
{
using boost::lambda::_1;
return std::find_if(begin, end, !_1);
}

And compare the return value to end().

Luke

 

[a.a.makarov]

(e-mail address removed) пиÑал(а):

Hi All,

I'm just getting started learning to use <algorithm> instead of loads
of little for loops, and I'm looking for a bit of advice/mentoring re:
implementing the following...

I have a vector of boost::shared_ptr's, some of which may be NULL. I'd
like to find out if there are any non-NULL elements in the vector.
Here's what I have at the moment.


typedef std::vector< boost::shared_ptr<::CFiCamera> > tCameraVector;
tCameraVector m_vpCameras;
...
...
bool CIIDCCaptureFacade::IsHardwareAvailable() const
{
//check for at least one initialized (non-NULL) camera
tCameraVector::const_iterator itCam = std::find_if(
m_vpCameras.begin(), m_vpCameras.end(),
std::bind2nd( std::not_equal_to<tCameraVector::value_type>(),
tCameraVector::value_type() )
);

return itCam != m_vpCameras.end();
}


Firstly, will this implementation meet the spec? I think it will.

Secondly, does anyone have any feedback on whether this is the 'right'
way to implement this using stl? Of course that's subjective, but I'm
looking for a little reassurance that I'm not getting into bad habits
right off the bat.

Cheers and TIA,

Pete

Hi.
Here is another solution.

#include <algorithm>
#include <boost/shared_ptr.hpp>

class some_class
{
private:
typedef std::vector<boost::shared_ptr<int> > container_type;
public:
// ...
bool null_exsists() const
{
return (std::find(cont_.begin(), cont_.end(),
boost::shared_ptr<int>()) != cont_.end());
}
// ...
private:
container_type cont_;
}

Best regards,
Andrey

 

[Pete Hodgson]

Thanks for the insightful response Luke, and for taking the time to help
me out. I do appreciate it!

template <typename T>
bool containsAnyNulls(std::vector< boost::shared_ptr<T> > const& vec)
{
using boost::lambda::_1;
return std::find_if(vec.begin(), vec.end(), !_1) != vec.end();
}

Caveat, I didn't take the time to test this code because you're just
asking about style. But it should work fine.

Firstly, AFAICT the above implementation will find the first NULL
element. I'm looking to find the first NON-null element.

Here's my revised implementation:

using boost::lambda::_1;
const boost::shared_ptr<::CFiCamera> nullCamera;

tCameraVector::const_iterator itCam = std::find_if(
m_vpCameras.begin(), m_vpCameras.end(),
nullCamera != _1
);

I like your suggestion of creating nullCamera to explicitly show we're
comparing to a null entry. I thought about using the bool casting
operator of boost::shared_ptr but I think using lambda and nullCamera is
clearer. Also, I couldn't figure out the syntax of getting the _1 cast
to a bool in the find_if(...) call!

Cheers,

Pete

 

[Roland Pibinger]

Here's my revised implementation:

using boost::lambda::_1;
const boost::shared_ptr<::CFiCamera> nullCamera;

tCameraVector::const_iterator itCam = std::find_if(
m_vpCameras.begin(), m_vpCameras.end(),
nullCamera != _1
);

Wrong group, try http://www.boost.org/more/mailing_lists.htm#users

 

[Markus Schoder]

Wrong group, try http://www.boost.org/more/mailing_lists.htm#users

boost is by many not considered off topic here since it is linked to
the standardization process of C++ and parts of it will very likely
make it into the next C++ standard revision.

We know what you think about boost so no need to repeat it.