What about memory leaks?

[SpeCter]

You could store an unique_ptr inside your map and return your raw pointer to whatever needs it.
The only difference is that you don't have to call delete yourself, SkillDirectory will do that for you the moment it leaves its scope.(Or to be more correct unique_ptr will do it because it leaves its scope)
The other classes can use the raw pointer like before but it is not their responsibility to delete the memory thats SkillDirectory's task and that will happen when you decide that its lifetime is over.
This would be a mix like ChronicRat said.

RAII to the rescue

[Jesper Juhl]

As I see it it's very simple.
When you are dealing with single ownership use unique_ptr since it offers many advantages over raw pointers and has zero overhead.
When you are dealing with shared ownership then shared_ptr is the obvious choice.
Only when you are dealing with non-owning pointers (or interfacing with C or legacy APIs) do raw pointers still make sense in a modern C++14 universe.

[Jabberwocky]

You could store an unique_ptr inside your map and return your raw pointer to whatever needs it.

That makes sense to me. 
I expected there would be more resistance to storing a secondary raw pointer to a unique pointer (edit - sounds like Jesper would be against this approach).  It's not really "unique" then.  But agree in this case it would work.

RAII to the rescue

I'm not totally sure how much in need of rescuing the pure raw pointer case needs.     You're basically trading a single destructor loop, for some more complicated pointer syntax.  But I appreciate your thoughts!  The unique_ptr implementation is completely reasonable to me.

[SpeCter]

It is unique, not in the sense that there is only one pointer pointing to an object though. Its name comes from the fact that it is the only thing owning whatever it points to.
unique_ptr and shared_ptr are only for ownership semantics, who owns the memory and who is responsible for releasing it.

[Jabberwocky]

Fully agreed, SpeCter.

I suspect some coders might be against it, solely because, in general, storing a raw pointer to another unique pointer creates a possible dangling pointer scenario.  Whereas it wouldn't if you used shared pointers. 

But yeah, there's no more "dangle danger" with the unique_ptr implementation than a pure raw pointer implementation.  And of course, part of the problem description is that these pointers would never go out of scope until game shutdown.

[SpeCter]

I don't understand what you mean with raw pointer to another unique pointer, could you make an example for me please?

[Rosme]

The issue now seems more to be on what people understand is unique_ptr. Some believe that means it is unique and it is the only place it should be. That is not the case. unique_ptr means unique ownership. Having a naked raw pointer obtain by unique_ptr::get() is fine, as long as the original owner of the unique_ptr does not go out of scope thus invalidating the raw pointer.

[Jabberwocky]

I don't understand what you mean with raw pointer to another unique pointer, could you make an example for me please?

class SkillDirectory
{
public:
   // ... constructors, etc...
   const Skill* LookupSkill(const std::string& i_skillName) const;

private:
   // maps a skill name to a skill class.
   std::map<std::string, std::unique_ptr<Skill*>> m_mapSkill;
};

const Skill* SkillDirectory::LookupSkill(const std::string& i_skillName) const
{
   // my syntax here is probably wrong because I don't use smart pointers, but...
   std::map<std::string, std::unique_ptr<Skill*>>::const_iterator it = m_mapSkill.find(i_skillName);

   if (it != m_mapSkill.end())
   {
      // return a raw pointer to the std::unique_ptr
      return (*it)->get();
   }
   return 0;
}
 

And some other gamesystem code stores this raw Skill* pointer like so:

bool PlayerSkills::AddSkill(const std::string& i_skillName)
{
   const Skill* pSkill = GetSkillDirectory()->Lookup(i_skillName);
   if (!pSkill)
   {
       // handle error..
       return false;
   }

   // Track this RAW SKILL POINTER in my "known skills" data.
   m_setKnownSkill.insert(pSkill);
   // So now the SkillDirectory has a unique_ptr to pSkill, and I have a raw pointer to pSkill.
}
 

[SpeCter]

Ah ok.
2 things to note here:
1. unique_ptr<Skill> instead of unique_ptr<Skill*>
2. without starting another war here, you should probably use null_ptr instead of 0 (or NULL)

As long as SkillDirectory doesn't get deleted there shouldn't be a problem.

[Jabberwocky]

Cool, so we're on the same page.

[Mörkö]

I too avoid smart pointers. Part of the reason is something Jonathan Blow said—one of very few things I agree with him about—namely that smart pointers changes the type of the object and that's not good. Illustration:

auto ptr = new Object{};

This is very easy to understand. `ptr` is simply the address of my object. The possible operations are also very limited (which is a good thing for stupid programmers like me). Passing it around is straight forward, and so is dereferencing it and performing arithmetic on it in those special cases. I feel like the object I'm handling is small and easy to understand.

auto ptr = std::unique_ptr<Object>(new Object{});

Now the type of the object has changed. It's no longer merely an address to my object, or even my object, but now I have an object of type "std::unique_ptr<..." which is something else than what I wanted. The new object I actually want sort of hidden somewhere inside it, both in terms of how unique_ptr works, and literally by how the declaration reads.

This increases the complexity of the program, because now I have to reason about how std::unique_ptr will act in addition to everything else. Mind you, this doesn't even absolve me from thinking about the lifetime of the object, it merely hides deallocation into the rules of scope instead of having me handle it explicitly. The reason why I use C++ is because I want to do memory management manually, if I wanted memory to be handled in automatic and hidden ways I would just a GC'd language instead.

I enjoy doing things explicitly, because that makes it easier for me to understand the meaning of my code. Someone may argue that smart pointers are actually super simple to understand, and if that's how you feel, then go ahead. However, if you are double freeing your pointers or forgetting to free them, or often trying to access freed memory, well I believe you have problems that smart pointers aren't going to solve. And like Jabberwocky said, if something like allocation fails then smart pointers aren't likely going to save you either.

[Rosme]

You clearly didn't read correctly all we said. Also, I don't get why you say it changes everything? unique_ptr offer the same operator(*, ->) to access the underlaying object. In fact, it makes the software more robust, because you are sure you are not going to copy the pointer because it is not copyable. The proof have been made by a lot of very experimented programmers(not only on this forum but across the internet) why smart pointers in general are good. But if you really believe that full manual memory management(because even with smart pointer, there is manual memory management) is really better, than I can't do anything but wait until you have an issue with your pointers. Now, here is another small example of memory leak if you want:

Foo* a = new Foo();
std::vector<Bar> myVector;

//Stuff happens, myVector is filled and is of size 10

auto& b = myVector.at(10); //Throws out_of_range

delete a; //Is never reached
 

Now remember this is a very trivial code, and most useless, but you can (hopefully) get the point. Unless you surround your call to at with try catch of course, you get a memory leak. Truthfully, you'll probably won't use at and use [] which will be UB instead. But you get the point. This is what full memory management can cause. It's problem, over problem. It's tricky and complicated. To fully make sure that there isn't any place where memory can leak, the code will look like world war three went through it. With std::unique_ptr, no worries.

[Jabberwocky]

Thanks for that, Mörkö.

Actually, I think you wrote your objections more eloquently than I did.

You clearly didn't read correctly all we said.

That isn't clear to me at all from Mörkö's post.  In fact, everything he said is technically correct, as far as I understand.

The proof have been made by a lot of very experimented programmers(not only on this forum but across the internet) why smart pointers in general are good.

I am a very experienced programmer.
Jonathan Blow is a very experienced programmer.
A simple google search lists many who critique them.
Obviously it's not as unanimous as you say.

But if you really believe that full manual memory management(because even with smart pointer, there is manual memory management) is really better, than I can't do anything but wait until you have an issue with your pointers.

Now, here is another small example of memory leak if you want:

There are other ways to find memory leaks in your program that do not involve permanent changes to your code at all.
Examples:
- valgrind
- this for visual studio

I can't do anything but wait until you have an issue with your pointers.

In my case, I'm afraid you're going to be waiting a long time.  Unless you honestly believe that every C++ program in existence before C++11 was a buggy piece of memory leaking crap (which is obviously not true), your statement doesn't hold.  I'll certainly agree with you if you say smart pointers can make the job of memory management easier for some programmers.  But it's blatantly incorrect to say they are necessary for a proper functioning program.

I do however very much appreciate hearing from folks who use smart pointers.  Maybe someday I'll change, or have to work on a project where they are code standard. 

[Mörkö]

You clearly didn't read correctly all we said. Also, I don't get why you say it changes everything? unique_ptr offer the same operator(*, ->) to access the underlaying object. In fact, it makes the software more robust, because you are sure you are not going to copy the pointer because it is not copyable.

It does change the type of the object. Using an object of type std::unique_ptr instead of simply (the address of) the object brings in a whole bunch of new rules and functionality which requires programmer attention. Just because it shares two of the operators doesn't change that fact.

About copying the pointer, see what I said about accessing freed memory. In that scenario you have written a program which you don't understand. Not being able to effectively reason about your own code is not a problem that smart pointers are going to solve, yes they might sweep some of the symptoms under the rug but they will not fix the underlying issue.

Now, here is another small example of memory leak if you want:

------ snipped ------

Now remember this is a very trivial code, and most useless, but you can (hopefully) get the point. Unless you surround your call to at with try catch of course, you get a memory leak. Truthfully, you'll probably won't use at and use [] which will be UB instead. But you get the point.

Why would you not wrap the thing that can throw in try/catch? Just exactly where is the catching clause located in the scenario you propose? If you don't catch it then it won't matter whether memory leaked. Your example doesn't have anything to do with smart pointers. I want to see some examples of robust and elegant code (i.e. doesn't look like ww3 went through it) where you use smart pointers for everything and let exceptions affect control flow in a major way.

This is what full memory management can cause. It's problem, over problem. It's tricky and complicated.

It's tricky (like programming generally), but I don't think it's complicated. Adding rules and functionality is what makes a program complicated, and that's what smart pointers do. They don't change the fundamental mechanism, they merely hide it behind another mechanism. Note that I am not denying they have any use at all, of course they do, but the trade-off in terms of complexity vs potential benefits is not worth it for me.

[Mörkö]

In my case, I'm afraid you're going to be waiting a long time.  Unless you honestly believe that every C++ program in existence before C++11 was a buggy piece of memory leaking crap (which is obviously not true), your statement doesn't hold.

Forget about C++ pre 11, think about plain C! Every C program must be a horrible leaking mess right?

Well I guess there are a significant portion of C++ programmers out there who would answer "YEAH, THAT'S RIGHT" ...but ignoring them I think there is a lot of evidence that low level programming was doing just fine even before smart pointers came along.