4

Ok, so everyone knows that raw pointers should be avoided like the plague and to prefer smart pointers, but does this advice apply when implementing a container? This is what I am trying to accomplish:

template<typename T> class AVLTreeNode {
public:
    T data;
    unique_ptr<AVLTreeNode<T>> left, right;
    int height;
}

Unique_ptr can make container functions more cumbersome to write because I can't have multiple raw pointers temporarily pointing to the same object in a way that is elegant. For example:

unique_ptr<AVLTreeNode<T>> rotate_right(unique_ptr<AVLTreeNode<T>> n1)
{
    unique_ptr<AVLTreeNode<T>> n2 = n1->left;

    n1->left = n2->right;
    n2->right = n1;
    // n1 must now be referenced through the longer name n2->right from now on
    n2->right->recalculate_height();
    n2->recalculate_height();

    return n2;
}

(It's not a big deal in this example but I can imagine how it could become a problem). Should I take problems like these as a strong hint that containers should be implemented with good old new, delete, and raw pointers? It seems like awfully a lot of trouble just to avoid writing a destructor.

Jeff Linahan
  • 3,775
  • 5
  • 37
  • 56
  • Except for substituting traditional include guards for pragma once, I've copied your code and do not replicate your error using clang/libc++. I suggest preprocessing driver.cpp, inspect that, and see if you see `unique_ptr` in it. The behavior you're seeing may be unrelated to `unique_ptr`, such as not including the C++11 version of ``. – Howard Hinnant Mar 29 '11 at 15:43

5 Answers5

7

I do not usually use smart pointers when implementing containers as you show. Raw pointers (imho) are not to be avoided like the plague. Use a smart pointer when you want to enforce memory ownership. But typically in a container, the container owns the memory pointed to by the pointers making up the data structure.

If in your design, an AVLTreeNode uniquely owns its left and right children and you want to express that with unique_ptr, that's fine. But if you would prefer that AVLTree owns all AVLTreeNodes, and does so with raw pointers, that is just as valid (and is the way I usually code it).

Trust me, I'm not anti-smart-pointer. I am the one who invented unique_ptr. But unique_ptr is just another tool in the tool box. Having good smart pointers in the tool box is not a cure-all, and using them blindly for everything is not a substitute for careful design.

Update to respond to comment (comment box was too small):

I use raw pointers a lot (which are rarely owning). A good sampling of my coding style exists in the open source project libc++. One can browse the source under the "Browse SVN" link.

I prefer that every allocation of a resource be deallocate-able in a destructor somewhere, because of exception safety concerns, even if the usual deallocation happens outside of a destructor. When the allocation is owned by a single pointer, a smart pointer is typically the most convenient tool in the tool box. When the allocation is owned by something larger than a pointer (e.g. a container, or a class Employee), raw pointers are often a convenient part of the data structure composing the larger object.

The most important thing is that I never allocate any resource without knowing what object owns that resource, be it smart pointer, container, or whatever.

Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577
  • Interesting. What other occasions (other than interfacing with C code) do you prefer raw pointers to smart pointers? – Jeff Linahan Mar 30 '11 at 22:19
  • @HowardHinnant Hi I'm more than 7 years late for the party. Do you still hold the same opinion for not using smart pointers for containers? I'm teaching myself Data Structure and it happens that every textbook I have at the moment (even new ones that definitely come out after smart pointers were introduced) are using raw pointers. I think it makes sense. – Nicholas Humphrey Jul 15 '18 at 17:45
  • @NicholasHumphrey Better late than never. ;-) My opinion on this subject is still about the same. I don't object to a container design that internally uses smart pointers as long as it's correct. But typically for my own code (and I don't code that many containers any more), I'll have the container do the owning instead of smart pointers. See [P0083](http://wg21.link/p0083) for a good example where ownership can transfer back and forth between a "smart pointer" (`node_handle`) and a container to achieve quite improved functionality. – Howard Hinnant Jul 15 '18 at 19:06
  • @HowardHinnant Thanks Howard, appreciate the explanation! – Nicholas Humphrey Jul 18 '18 at 20:45
3

The code you presented compiles with no problems

#include <memory>
template<typename T> class AVLTreeNode {
public:
    T data;
    std::unique_ptr<AVLTreeNode<T>> left, right;
    int height;
};
int main()
{
    AVLTreeNode<int> node;
}

test compilation: https://ideone.com/aUAHs

Personally, I've been using smart pointers for trees even when the only thing we had was std::auto_ptr

As for rotate_right, it could be implemented with a couple calls to unique_ptr::swap

Cubbi
  • 46,567
  • 13
  • 103
  • 169
  • that... was strange. VC++ compiled what you have just supplied but rejected my implementation. I have edited the question to include my code, help on why it doesn't compile would be appreciated. – Jeff Linahan Mar 29 '11 at 15:29
  • @da_code_monkey something's up with VC++. Your full example compiles with gcc 4.5.2, but it only compiles on VC++ 2010 EE if I add `AVLTreeNode dummy;` in the beginning of `main`, before AVLTree a; – Cubbi Mar 29 '11 at 15:48
0

Herb Shutter has very clear guideline about not using shared_ptr as parameters in his GoTW series:

Guideline: Don’t pass a smart pointer as a function parameter unless you want to use or manipulate the smart pointer itself, such as to share or transfer ownership.

and this...

Guideline: Prefer passing objects by value, *, or &, not by smart pointer.

Shital Shah
  • 63,284
  • 17
  • 238
  • 185
0

Small correction: raw pointers should not be avoided like the plague (oops, not everybody knew the fact), but manual memory management should be avoided when possible (by using containers instead of dynamic array or smartpointers), so in your function, just do a get() on your unique_ptr for temporary storage.

stefaanv
  • 14,072
  • 2
  • 31
  • 53
0

std::shared_ptr does not have these restrictions. Especially, multiple shared_ptr-instances can reference the same object.

Björn Pollex
  • 75,346
  • 28
  • 201
  • 283