unique_ptr for ownership

[hrobeers]

Since you're using C++11, wouldn't it be better to manage object ownership using unique_ptr?

I will issue a pull request when I start using your library.

[nlohmann]

As I wrote on Reddit:

There is no specific reason. I used pointers in order to limit the union's size. As the pointers are never exposed to the user, I never thought about using smart pointer. However, this could be a nice experiment. Do you know whether there are size overheads compared to raw pointers?

When there is no overhead (i.e. the size of the union will not grow), I would change the code myself.

[hrobeers]

Let's quote Reddit again ;)

[–]hrobeers 3 points 13 hours ago
If I'm not mistaken, there is no memory overhead because it just stores the raw pointer internally. But there could be a little runtime performance overhead when using another deleter. So when using the default deleter instead of manually call delete, there is no overhead.

[–]Splanky222 3 points 12 hours ago
No overhead with regular destructors. Unique_ptr is just a wrapper around a raw pointer so you can use RAII

[hrobeers]

This would also fix issue #11

[nlohmann]

Alright - I understand that using unique_ptrs will be really nice. However, it seems as if the situation where pointers are inside a union makes the code quite difficult to port to unique_ptr in case we start swapping and moving. At lease this is what I take away from http://stackoverflow.com/questions/24713833/using-a-union-with-unique-ptr

I may need some more time to really understand the problem, but currently it seems as if I haven't understood all the implications yet. If you have any helping reference, I would be glad.

[hrobeers]

Yes it is indead true that smart pointers cannot be used inside unions. I missed the fact that you were heavily relying on a union, so using unique_ptrs is not possible.
When I start using your library, I'll try to find some other ways to manage memory.

Thanks for sharing this code, it looks really nice!

[nlohmann]

Thanks! Until then, we have to rely on Valgrind that no memory is leaked ;-)

[cmaloney]

For the json::iterator and json::const_iterator you can use optional types (Like std::optional which was unfortunately dropped from c++14), as a replacement for the explicit new / delete calls. Also can give a slight speed boost because it eliminates a indirection. std::unique_ptr would work as well though / take less additional boilerplate.