• Advertisement
Sign in to follow this  

Returning boost::shared_ptr

This topic is 3949 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, I've been persuaded to use boost::shared_ptrs instead of standard pointers but have wasted alot of time with these time-saving helpers. My newest problem is this: With standard pointers, a function may look like this:

CMyClass* myClass;

CMyClass* ReturnClassOrNull( bool b )
{
   if ( b )
      return myClass;
   else
      return NULL;
}

This ability to return a NULL was very useful, since it can be easily checked for validity. However this version produces an error, since a boost::shared_ptr is actually an object, not a pointer:

boost::shared_ptr<CMyClass> myClass;

boost::shared_ptr<CMyClass> ReturnClassOrNull( bool b )
{
   if ( b )
      return myClass;
   else
      return NULL;
}

Is there any way a shared_ptr can return the equivalent of a NULL? Thanks Si

Share this post


Link to post
Share on other sites
Advertisement
Yes. Example:(I prefer to typedef smart pointers so I don't write the same things over and over):


typedef boost::shared_ptr<CMyClass> PMyClass;
PMyClass myClass;

PMyClass ReturnClassOrNull(bool b)
{
if (b)
return myClass;
else
return PMyClass();
}



Share this post


Link to post
Share on other sites
I use a shared_ptr instanciated with the default constructor instead of NULL.

shared_ptrs can be tested for truth (eg if(mysptr) ) in the same way as pointers.

I believe this is the intended use. Also, you'll get an assertion failure if you attempt to dereference a default sptr.

edit: oops, didn't read mikeman's response properly.

Share this post


Link to post
Share on other sites
I find it ugly to write "return boost::shared_ptr<T>()" so I usually use a helper class:


const struct null_ptr_t
{
template <typename T>
operator boost::shared_ptr<T>() const
{
return boost::shared_ptr<T>();
}
} null_ptr;

boost::shared_ptr<int> foo()
{
return null_ptr;
}

boost::shared_ptr<SomeOtherType> bar()
{
return null_ptr;
}

int main()
{
boost::shared_ptr<int> a = foo();
boost::shared_ptr<SomeOtherType> b = bar();
}

Share this post


Link to post
Share on other sites
Quote:
Original post by sipickles
I've been persuaded to use boost::shared_ptrs instead of standard pointers but have wasted alot of time with these time-saving helpers.
Since I think I was the one who suggested managing your resources via shared_ptr, I should probably comment on this :)

If you're new to smart pointers and/or the Boost libraries it might take a little time to get used to the semantics and the finer points of usage, but I'm sure it will pay off in the long run.

Problems with passing 'standard' raw pointers around include:

1. You have to remember to delete them.
2. You can delete them at any time, even if 'live' pointers to the same object still exist elsewhere in the program.
3. There's no way to check a non-null pointer for validity - you just have to dereference it and hope for the best.

I think the 'wrapper' class you had in place previously was intended to address some of these problems, but shared_ptr is tried and tested and addresses all of the problems listed above. (Just to be thorough, you can still get yourself in trouble through careless use of the get() function, but this more or less falls into the category of 'deliberate misuse'.)

Also, as noted above shared_ptr shares similar semantics with raw pointers in several important respects. You can check them for validity via Boolean expressions, and you can have a 'null' shared pointer object that represents an 'empty' or unassigned pointer.

shared_ptr offers additional useful functionality as well; for example, at any time you can query a pointer to find out how many times the held object is referenced throughout the program (provided you've used smart pointers consistently).

Boost also has a few other useful smart pointer classes that 'fill in the gaps', so to speak. weak_ptr can be used to break cyclic dependencies, and scoped_ptr can be used where sole ownership is desired. (However, AFAIK scoped_ptr cannot easily be used with resources such as the DirectX effects under discussion in your previous thread, as it does not offer support for custom deleters).

So yes, when adopting a new tool there will probably be a little 'bump' and a little extra time spent implementing features that may at first seem like they were 'easier' the old way. Nevertheless, management of dynamically allocated resources via smart pointers represents the current best practice in C++ (or so I gather - I hope the resident gurus will correct me if I'm wrong in this respect), and should be preferred over the use of raw pointers where possible. In the end, it should save you more time (in terms of easier maintenance and fewer bugs) than it costs.

Share this post


Link to post
Share on other sites
Thanks to everyone for the help, particularly jyk for the eloquent, encouraging and ultimately persuasive posting. :)

-----------------------------------------------

All my reworking of my original problem is still leading me to the same problem. I am trying to get my resource manager template to create a new resource and return a boost::shared_ptr to it.

Unfortunately, I come unstuck when I try to create the resource in the template resourceManager:


// Type: boost::shared_ptr< CEffect >

// std::map< std::string, Type > m_list;

// Create the resource
Type resource( new Type( name, path ) );

// Add the new resource to the manager and return a smart_pointer to it.
m_list[name] = resource;
return m_list[name];




The create resource line is meant to create a new CEffect, but the type is boost::shared_ptr< CEffect >, so I get an error (wrong constructor).

How do I actually allocate the object in this template? Its all abstracted one level....

Thanks


[Edited by - sipickles on March 28, 2007 2:42:30 PM]

Share this post


Link to post
Share on other sites
Your problem is here:
Type resource( new Type( name, path ) );

Since Type is shared_ptr, you're trying to construct a shared_ptr with the CEffect's constructor and then copy that into a second shared_ptr.

Solution:
Type resource = new Type::element_type(name, path);

shared_ptr::element_type resolves to the type pointed to by shared_ptr, which in this case is CEffect.

(Please note that this is off the top of my head so the precise syntax for getting at element_type may not be right; I don't do it very often.)

Share this post


Link to post
Share on other sites
Ah, thanks

I think I solved it another way. I create the template using my object type, then in the resource manager use boost::shared_ptr< Type >. The offending line then becomes


// Create the resource
boost::shared_ptr<Type> resource( new Type( name, path ) );



Your way is neater tho!

Share this post


Link to post
Share on other sites
So it continues... now the problem is with shared_ptr::reset()


bool Remove( std::string name )
{
// Iterate the list looking for the specified resource.
std::map< std::string, boost::shared_ptr< Type > >::iterator iter;// = m_list.begin();

for ( iter = m_list.begin(); iter != m_list.end(); ++iter )
{
if( name.compare( iter->first ) == 0 )
{
g_log->SIM( "Removing reference to %s (%d references)\n", const_cast<char*>(iter->second.get()->GetName().c_str()), iter->second.use_count() );
iter->second.reset();
if ( iter->second.use_count() == 0 )
{
g_log->SIM( "Resource %s now deleted, removing from ResourceManager: \n", const_cast<char*>(name.c_str()) );
m_list.erase( name );
g_log->SIM( "ResourceManager has %d resources left\n", m_list.size() );
}
return true;
}
}

g_log->SIM( "ERROR - Resource %s not found during ResourceManager::Remove()\n", const_cast<char*>(name.c_str()) );
return false;
}


When the above function is called, there are 3 refs to the CEffect in question. Calling:

iter->second.reset()

nulls the .second part of the std::map. Fine. My problem is, how can I check if the object is still in use elsewhere (has it been automatically destructed?) coz if it has, I need to remove it from my std::map.

Thanks again

Share this post


Link to post
Share on other sites
It looks there are some tweaks that could still be made here, so I'll go ahead and offer a couple more comments...

In looking at your Remove() function, I think the first question to ask is, what behavior do you intend for it to have?

Presumably if the resource for which removal has been requested is not currently referenced outside of the manager (that is, the manager 'owns' the only copy), then you want to delete the object and remove it from the map.

If however the manager does not own the only copy, you have a decision to make. Do you log a message of some sort and leave the resource in the manager? Or do you go ahead and remove it from the manager and let whoever else has it retain ownership? (I don't know that there's one 'right' answer to this question; you just have to decide what you want to have happen, and then implement accordingly).

Given all that, I see a couple of things in your current implementation that might be worth a second look. Here are some key points to keep in mind:

1. Assuming proper usage, the 'held' resource will not be deleted until the last shared pointer referencing the resource is reset or destroyed. Furthermore, this destruction is automatic.

2. As I understand it, the use_count() function (and, by extension, unique()) is intended to be called only on non-empty shared pointers. To be more specific, the Boost documentation states that when called on an empty shared pointer, use_count() returns 'an unspecified nonnegative value'. In other words, calling use_count() after calling reset() provides no useful information.

I think your intent here is to determine whether the manager currently has sole ownership of the resource, and take action accordingly. I can't comment on the specific actions in question without knowing what the expected behavior of the function is, but to determine uniqueness you should call the function unique() before modifying the shared pointer in any way (e.g. via reset()). This will tell you whether other smart pointers also reference the object.

With that in mind, here's your code excerpt, modified and with a few comments:

bool Remove( std::string name )
{
std::map< std::string, boost::shared_ptr< Type > >::iterator iter;

for ( iter = m_list.begin(); iter != m_list.end(); ++iter ) {
if( name.compare( iter->first ) == 0 ) {

// Found the resource. Check to see if we have sole ownership:
if (iter->second.unique()) {
// Do something (for example, erase the entry and thereby free the resource).
} else {
// We don't have sole ownership. What to do?
}
}
}
}

(There are a few other minor stylistic issues at play here, but I'm just focusing on smart pointer usage for now.)

Anyway, I hope I'm doing more than just adding to the confusion here :| Although there's not that much to the Boost smart pointer classes, it can take a little work to get a firm grasp on all aspects of their behavior.

Share this post


Link to post
Share on other sites
Jyk, true ambassador for boost!

The leap from standard pointers isn't that bad in the end, just subtle. I am happy to have tackled it finally. It is very helpful.

One small point. Sometimes the automatic destruction of things happens in an unhelpful (ie uncontrolled) order. I've kept to using standard pointers for big engine features so I can control the order of shutdown.


Thanks for all your help Jesse. I am, of course, now intrigued by this:

Quote:

(There are a few other minor stylistic issues at play here, but I'm just focusing on smart pointer usage for now.)


Care to elaborate? I am on the cusp of embarking on a big engine consumation of many elements. Any general coding guidelines much appreciated at this stage.

Regards

Si

Share this post


Link to post
Share on other sites
Quote:
Original post by sipickles
One small point. Sometimes the automatic destruction of things happens in an unhelpful (ie uncontrolled) order. I've kept to using standard pointers for big engine features so I can control the order of shutdown.

There's a truly elegant way to handle this too: If module A depends on module B (in terms of B not being able to shut down before A is shut down) simply have A hold a shared_ptr to B.

Share this post


Link to post
Share on other sites
Quote:
Original post by sipickles
Care to elaborate? I am on the cusp of embarking on a big engine consumation of many elements. Any general coding guidelines much appreciated at this stage.
Here are a couple more notes (nothing comprehensive):


bool Remove( std::string name )
{
// Whenever you find yourself slogging though awkward template
// syntax by hand, consider using a typedef. (This also makes it
// easier to change types later on.)
std::map< std::string, boost::shared_ptr< Type > >::iterator iter;

// Whenever you find yourself writing out a loop by hand, consider
// using for_each() instead. It's not always applicable (and in fact
// is not really applicable here), but in general it makes for code
// that's easier to read and debug.

// Next, when you find yourself *searching* for an item in a container,
// consider using the find() algorithm instead. Furthermore, std::map
// has its own 'find' function (which is probably what you should be using
// here).

// Another tip that's not directly applicable here but is good to keep
// in mind nonetheless is to use remove() or remove_if() along with
// erase() to remove items from sequential containers such as lists and
// vectors.

// The general theme here is, anytime you find yourself implementing
// some common bit of functionality 'by hand' (this can include anything
// from inserting the contents of one container into another,
// to streaming the contents of a container to the console), stop for a
// moment and take a quick look through a good SC++L reference, or the Boost
// documentation. More often than not you'll find that someone else has already
// done the work for you.

// Note: I usually refer to the online SGI docs for info on the STL-
// derived portions of the SC++L. Also, a .pdf summarizing the algorithms
// from the C++ standard library can be found here:

// http://www.josuttis.com/libbook/algolist.pdf

for ( iter = m_list.begin(); iter != m_list.end(); ++iter )
{
// If you use std::string consistently, you should be able to compare
// strings directly using operator==(). If you need case-insensitive
// comparison, you can use boost::iequals().
if( name.compare( iter->first ) == 0 )
{
// It looks like you're using an 'old-school' logger that uses
// variadic macros. Although I can imagine it may not be convenient
// to change this at this time (depending on how extensively this
// class is used throughout your code), you might reconsider it when
// you start your next project.

// In short, anytime you find yourself writing something like
// 'const_cast<char*>(...c_str())', you should be a little alarmed :)
// There are safer and more idiomatic ways to handle formatting
// in C++ that, in general, should be preferred over variadic
// functions.
g_log->SIM( "Removing reference to %s (%d references)\n", const_cast<char*>(iter->second.get()->GetName().c_str()), iter->second.use_count() );
// ...
}
}
}

Share this post


Link to post
Share on other sites
Wow, these are words of a programmer with many years more experience than me.

I do things by hand, then always discover someone has already done it (better!). Still, there's some merit in understanding the process involved by DIY, then using the optimised functions some whizz-kid has developed.

Christ, I don't even know what a 'variadic macro' is!!!!! or 'idiomatic' for that matter!

The const_cast<char*>...c_str() business was a stopgap after using char* in all my classes.

I've upgraded everything to std::strings, but had a problem with my logging class.

I'd rather have CLog::Log( std::string s ), but how do you write in variables? Manually building the std::string? whats the point?

There must be a better way!

Thanks to all your advice, jesse, some of my code is becoming more concise and readable. I am glad you can't see the rest! :D

Si

Share this post


Link to post
Share on other sites
Quote:
Original post by sipickles
Wow, these are words of a programmer with many years more experience than me.
Ha. You want to talk about experience and wisdom, just read some of the posts by the real gurus on these forums :|

Thanks though :)
Quote:
Still, there's some merit in understanding the process involved by DIY, then using the optimised functions some whizz-kid has developed.
Agreed. I think the best path is in the middle somewhere. Doing everything yourself is just masochistic, but if on the other hand you skip over all the low-level stuff, you'll find it more difficult to use the higher-level tools that we're fortunate enough to have available.
Quote:
I've upgraded everything to std::strings, but had a problem with my logging class.

I'd rather have CLog::Log( std::string s ), but how do you write in variables? Manually building the std::string? whats the point?
You'll see many different approaches to loggers, many of which include numerous features such as multiple targets, filters for messages of different types and severity levels, support for HTML or other formats...the list goes on.

I don't know what all your logger class does, but an option you might consider (at least for the time being) is simply to stream everything to std::cerr or std::clog as appropriate. No muss, no fuss, and you get all the features of the SC++L stream classes for free.

If for some reason though you're stuck with a logger that expects a single string, you can get variadic argument-like behavior (but with greater safety) by using Boost Format.

Share this post


Link to post
Share on other sites
Quote:
Original post by sipickles
I do things by hand, then always discover someone has already done it (better!).


Of course. There are millions of people out there doing things. Someone's implementation is bound to be better. ;)

Quote:
Christ, I don't even know what a 'variadic macro' is!!!!!


It's a macro which is variadic, i.e. accepts a varying number of parameters. Here, SIM is assumed to be a variadic macro which in turn expands into a call of a variadic function. These are things like the C printf(), which are not type-safe, cannot be made to work properly with non-POD types (AFAIK) and are in general very dangerous.

Quote:
or 'idiomatic' for that matter!


This is an English term, not (merely) a programming one; look it up. :)

Quote:
The const_cast<char*>...c_str() business was a stopgap after using char* in all my classes.

I've upgraded everything to std::strings, but had a problem with my logging class.

I'd rather have CLog::Log( std::string s ), but how do you write in variables? Manually building the std::string? whats the point?


1) the correct stopgap would have been to change the function prototype to accept const char*. After all, a log function shouldn't alter the text that's passed to it, right? That's what const correctness is about - advertising your promises not to change data. This gives you extra information which helps you reason about program correctness (and helps the compiler warn you).

2) Speaking of which, parameters of object type, such as std::string, should normally be passed by const reference.

3) Instead of "writing in" variables, a common approach is to stream them. You can easily design an object that "works like" a std::ostream by overloading its operator<< (such that it forwards to an actual underlying stream, for example) and having that return a reference to self (i.e., '*this'). Another approach (which I commonly use) is to have each "component" enclosed in parentheses - you do this by letting the first set be the argument for a one-argument constructor of a temporary object, and the subsequent ones be calls to the object's operator(), which is written to chain in the same way. If you need to indicate some kind of "flush" for your logger object in that scheme, you can either use a no-arg operator(), or do it in the object destructor. :)

Or, if you really like that % syntax, you could go with boost::format. It's typesafe, through the magic of templates (I think it relies on boost::tuple underneath), and doesn't require you to match up letters to types (because there would be no reason for it). I think it also supports positional arguments (e.g. "the %1 %0", "dog", "purple"), which is incredibly convenient for i18n.

[Edited by - Zahlman on March 30, 2007 11:57:14 PM]

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement