0

I am trying to refactor some oldish code, and I want to use unique_ptrs for some objects where they are clearly suited. Up till now, shared_ptrs have been generally used.

Given that for most intents and purposes both smart pointers behave identically, in many cases I don't see why I should have to distinguish between the two. So to make a trivial example:

EDIT: I've had to make the object a little less trivial...

class NamedItem
{
    string name;
    string& GetName();
}

class SessionObject: public NamedItem
{}

class TrivialObject: public NamedItem
{}

class NameCacher:
{
    vector<??????<NamedItem>> named_items;
    void AddNamedItem(??????<NamedItem>& named_item)
    {
        named_items.push_back(named_item);
    }
    void PrintAllNamedItems()
    {
        // Print all names
    }
}

unique_ptr<SessionObject> session(new SessionObject("the session"));
shared_ptr<TrivialObject> some_object(new TrivialObject("whatever"));

NameCacher names();

names.AddNamedItem(session);    // The session pointer will not delete the session object, even if names stops referencing it.
names.AddNamedItem(some_object);    // The some_object pointer is welcome to delete itself if names stops referencing it and nothing else is.

names.PrintAllNamedItems();

// If some_object goes out of scope, then its shared_ptr will delete it at this point.

Given that 80% of the day-to-day behaviour of the smart pointers is the same, isn't there a way to do this? The only thing I've found is to convert a unique_ptr to a shared_ptr - which is categorically not what I want to do. Ideally, I'd like the base class of the two smart pointers - but I can't find one.

Mike Sadler
  • 1,750
  • 1
  • 20
  • 37
  • 6
    A constant reference to a smart pointer as a function parameter usually doesn't make much sense. If this parameter is not related to the ownership of the object, simply pass it by a constant reference (`const NamedItem&` in your case) and then call `PrintName(*session)`. – Daniel Langr Apr 07 '22 at 09:17
  • 4
    Functions should not accept a smart pointer unless they need to modify (or copy/move etc) the smart pointer itself. Change the functions to accept raw pointers or references. – Galik Apr 07 '22 at 09:21
  • I did rather simplify this a lot - in reality, the function keeps a copy of the pointer and does all manner of things to it. I thought that this example would highlight a case where the risk is absolutely zero. – Mike Sadler Apr 07 '22 at 09:24
  • 3
    If it is taking a copy of the pointer you can't use `unique_ptr`? – Alan Birtles Apr 07 '22 at 09:34
  • It is my understanding that if you convert a unique_ptr to a shared_ptr, it transfers ownership, and the recipient can delete it. – Mike Sadler Apr 07 '22 at 09:42
  • 3
    But why do you need the smart pointer in the function? What are you planning to do to it? – Galik Apr 07 '22 at 09:43
  • OK, I've made the example slightly closer to the true situation. My basic point is that for 99% of its use, there is no difference between the smart pointers - the difference is how the smart pointers themselves choose to delete the memory they own. – Mike Sadler Apr 07 '22 at 09:59
  • @Galik other people tell you to never ever use raw pointers – user253751 Apr 07 '22 at 10:03
  • 2
    If `named_items` doesn't take ownership of the pointer then you can't use `unique_ptr`, just use raw pointers – Alan Birtles Apr 07 '22 at 10:03
  • 6
    @user253751 _"other people tell you to never ever use raw pointers"_ You sholdn't listen to those people. You should avoid naked `new` and `delete`. But raw pointers should be used. Even the C++ Core Guidelines tell you to pass raw pointers when you don't transfer ownership: [F.7: For general use, take T* or T& arguments rather than smart pointers](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f7-for-general-use-take-t-or-t-arguments-rather-than-smart-pointers) – jabaa Apr 07 '22 at 10:04
  • 3
    Have you determined that using `std::shared_ptr` everywhere has unacceptable overhead or is there some other factor driving your wish to avoid it in some circumstances? – Paul Sanders Apr 07 '22 at 10:11
  • @PaulSanders we felt that a unique_ptr was a more accurate intention for our "session" object - but I think that your inference is correct: if I use a shared_ptr my problem will be solved... – Mike Sadler Apr 07 '22 at 10:14
  • 4
    Ok. But I'm with @Galik - pass a raw pointer when appropriate. That would, IMO, adequately express intent. And make it `const` when you can. – Paul Sanders Apr 07 '22 at 10:16
  • 6
    Why do you need smart pointers at all? Do you transfer ownership or extend lifetime? You should read my link to the Core Guidelines. Passing a smart pointer when not needed is an example of bad code. – jabaa Apr 07 '22 at 10:16
  • @jabaa yes, there are a number of real equivalents of "NameCacher" which all have a legitimate interest in the objects. I want an object to be cleared up when none of these owners reference an object. I believe this is a classic use case of shared_ptrs - but I am trying to throw a couple of unique_ptrs into the mix. I am now thinking this is just bad practice... – Mike Sadler Apr 07 '22 at 10:25
  • Passing smart pointers around is bad practice, yes. I believe this is a classic use case of raw pointers. – jabaa Apr 07 '22 at 10:28
  • @jabba that wasn't what I meant - if shared_ptrs can't be passed around, then what is their use? The point is that they *can* be passed around, and when the last reference to them is dropped, the pointer frees up the memory. – Mike Sadler Apr 07 '22 at 10:40
  • You can pass around shared pointers to transfer ownership or extend lifetime. You should pass around raw pointers whenever you don't transfer ownership or extend life. Of course, technically you can always pass around shared pointers, but this will usually yield bad code. I often see this behavior in code of people coming from the Python or Java world with less experience in C++. – jabaa Apr 07 '22 at 10:44
  • @jabaa yes - that is what the real code does. Most of the objects are created in factories (which relinquish ownership immediately) and are handed to a set of managers which have various lifetimes. Once the last manager with a reference to it is destroyed, the object should be cleared up. – Mike Sadler Apr 07 '22 at 10:47
  • 2
    This sounds like your code needs a refactoring. – jabaa Apr 07 '22 at 10:49
  • @jabaa of that there is no doubt - but occasionally I believe we do use shared_ptrs correctly! I don't understand your blanket dismissal of passing shared_ptrs - because if a shared_ptr isn't shared, it has no purpose. – Mike Sadler Apr 07 '22 at 10:54
  • 1
    It's impossible to decide or make a recommendation for your actual project. In the [mcve] in your question, I would recommend raw pointers. – jabaa Apr 07 '22 at 10:58
  • 3
    You are already drowning in a deluge of advice (some conflicting). My suggestion is to just consider what ownership you need. From you example and the comments, it seems you need shared ownership. If that's the case, just accept shared pointers (by value). Please do not accept lvalue references to unique_ptr's. Especially if there's modification of the source pointer involved. It defeats the purpose of those handles, and the fact they make ownership semantics explicit via needing explicit moves from lvalues. – StoryTeller - Unslander Monica Apr 07 '22 at 11:05
  • @StoryTeller-UnslanderMonica I think you've just summarised the conclusion to which I was arriving at! I've tried to summarise it in an answer, just to distil my learning for future readers. Maybe you could check it? – Mike Sadler Apr 07 '22 at 11:09
  • 3
    Shared pointers are usually for when you have 2 or more objects in the code that need to refer to the same thing, and you do not have any control over which order they are deleted. So you use a shared pointer to keep track of the last reference to delete it for you. They are not intended to be passed around the system as some kind of heavy-weight "super safe" pointer. Memory should be managed by design and by placing your smart pointers in the appropriate scope and then "calling down" into functions passing raw-pointers or references, knowing the smart pointer will outlive the function call. – Galik Apr 07 '22 at 11:44
  • 1
    @Galik yes, that is a good description of how they are being used here. They are created by a factory, then passed to several managers. The destruction order of the managers is not defined, so the shared_ptr will free up the memory once the last manager that references the objects is destroyed. – Mike Sadler Apr 07 '22 at 12:25

1 Answers1

0

Many thanks to all those who have responded to my question. I've been speaking with a knowledgeable colleague as well, and it's taken us about an hour to get a common understanding of the whole situation - so my apologies for not being able to convey this in my simplified example.

I thought I'd add this as an answer to explain to future readers why the premise of my question was ill-conceived. This attempts to summarise some of the comments on the original question.

I believe that I had misunderstood the utility of unique_ptrs, and had been using them incorrectly. What I had originally wanted was a pointer that behaved as a shared_ptr does, but did not need to manage its reference count - because I could guarantee that it would stay alive for the entire session. As such, the code which referenced it could treat it the same as a normal shared pointer - it is just didn't need to increment or decrement the reference count.

However, the purpose of a unique_ptr is that it's ownership can be transferred - and in my example above I am attempting to send it to another object while not transferring its ownership. As several commenters pointed out, this could be done much better by dereferencing it as a raw pointer or a reference as it is given to the recipient - but this would be a very different intention to when a shared_ptr is provided. As such, they shouldn't have a common interface as I had originally asked.

My thanks again for everyone who helped me understand my mistake.

Mike Sadler
  • 1,750
  • 1
  • 20
  • 37