Which kind of (auto) pointer to use?

Question

I came accross several questions where answers state that using T* is never the best idea.

While I already make much use of RIIC, there is one particular point in my code, where I use T*. Reading about several auto-pointers, I couldn't find one where I'd say that I have a clear advantage from using it.

My scenario:

class MyClass
{

  ...
  // This map is huge and only used by MyClass and 
  // and several objects that are only used by MyClass as well.
  HashMap<string, Id> _hugeIdMap;
  ...
  void doSomething()
  {
    MyMapper mapper;
    // Here is what I pass. The reason I can't pass a const-ref is
    // that the mapper may possibly assign new IDs for keys not yet in the map. 
    mapper.setIdMap(&_hugeIdMap);
    mapper.map(...);
  }
}

MyMapper now has a HashMap<...>* member, which - according to highly voted answers in questions on unrelated problems - never is a good idea (Altough the mapper will go out of scope before the instance of MyClass does and hence I do not consider it too much of a problem. There's no new in the mapper and no delete will be needed).

So what is the best alternative in this particular use-case?

Monogenic

So you explained why you can't pass a const reference, but why can't you pass a reference? The only reason to use a pointer argument instead of a reference is if NULL is a valid value (i.e. the argument is optional). – Dou

Generally, use std::shared_ptr or std::tr1::shared_ptr. In C++0x you could probably use std::unique_ptr if you only have one responsible container and don't need to pass the pointer around. – Angelangela

@Sven: Also you cannot assign references, which the presence of setIdMap suggests. You'd have to rewrite the MyMapper class to take a reference in the constructor. But of course that would be a far nicer solution if it worked. – Angelangela

There is no need to use the HashMap as a pointer, because even a large HashMap has a small footprint on the stack: it stores most of its data on the free store. – Coltin

@John: that is not entirely true. Passing a large HashMap by value will likely invoke its non-trivial copy ctor, potentially copying a lot of data. – Dou

@Sven: That's mainly because our coding standards have a convention that says never pass non-const references. While I don't want to argue about how good those rules are, the reason is that clients of "MyMapper", in this case, will definately have to expect changes to objects whose pointer they pass, while passing something by reference may turn out to be highly confusion if they don't expect the object to be changed. – Monogenic

@b.buchhold: fair enough. As I said in my answer, I think a naked pointer is okay here since MyMapper is not managing the lifetime of the object. – Dou

@Sven: I didn't say pass it by value. That would be bad. I said the stack footprint is small. – Coltin

@John: That's exactly why it is okay to use a non-pointer as member in MyClass. However, the interesting point is how to pass it to MyMapper. Passing the object (no prt, no reference) will invode the non trivial copy constructor and consume time & space. So i figured, the only possibilities in MyMapper are using a pointer (discouraged in other answers), a references (nice like sven proposes but forbidden by our conventions) and some kind of smart_ptr (but which one and why?) – Monogenic

Answer 1

Personally I think a raw pointer (or reference) is okay here. Smart pointers are concerned with managing the lifetime of the object pointed to, and in this case MyMapper isn't managing the lifetime of that object, MyClass is. You also shouldn't have a smart pointer pointing to an object that was not dynamically allocated (which the hash map isn't in this case).

Personally, I'd use something like the following:

class MyMapper
{
public:
    MyMapper(HashMap<string, Id> &map)
        : _map(map)
    {
    }
private:
    HashMap<string, Id> &_map
};

Note that this will prevent MyMapper from having an assignment operator, and it can only work if it's acceptable to pass the HashMap in the constructor; if that is a problem, I'd make the member a pointer (though I'd still pass the argument as a reference, and do _map(&map) in the initializer list).

If it's possible for MyMapper or any other class using the hash map to outlive MyClass, then you'd have to start thinking about smart pointers. In that case, I would probably recommend std::shared_ptr, but you'd have to use it everywhere: _hugeIdMap would have to be a shared_ptr to a dynamically allocated value, not a regular non-pointer field.

---

Update:

Since you said that using a reference is not acceptable due to the project's coding standards, I would suggest just sticking with a raw pointer for the reasons mentioned above.

Dou

@iammilind: fine, whatever. :P – Dou

"Since you said that using a reference is not acceptable due to the project's coding standards, I would suggest just sticking with a raw pointer" - if the coding standard just says don't pass by non-const reference, you could stick with a non-const reference member, but the constructor can take a pointer - MyMapper(HashMap<string, Id> *pmap) : _map(*pmap) {}. – Maximilien

Answer 2

Naked pointers (normally referred to as raw pointers) are just fine when the object has no responsibility to delete the object. In the case of MyMapper then the pointer points to an object already owned by MyClass and is therefore absolutely fine to not delete it. The problem arises when you use raw pointers when you do intend for objects to be deleted through them, which is where problems lie. People only ask questions when they have problems, which is why you almost always see it only used in a problematic context, but raw pointers in a non-owning context is fine.

Matelot

Answer 3

How about passing it into the constructor and keeping a reference (or const-reference) to it? That way your intent of not owning the object is made clear.

Passing auto-pointers or shared-pointers are mostly for communicating ownership.

• shared pointers indicate it's shared
• auto-pointers indicate it's the receivers responsibility
• references indicate it's the senders responsibility
• blank pointers indicate nothing.

About your coding style:

our coding standards have a convention that says never pass non-const references.

Whether you use the C++ reference mechanism or the C++ pointer mechanism, you're passing a (English-meaning) reference to the internal storage that will change. I think your coding standard is trying to tell you not to do that at all, not so much that you can't use references to do so but that you can do it in another way.

Sabadilla

I think the coding standard is just saying to use pointers in the case where the referand can be modified. The reason is that pass-by-reference and pass-by-value look exactly the same at the call-site, and some people don't want to have to check the function signature in order to assume something that looks like pass-by-value, doesn't modify the argument. That's the justification I've seen when the rule is made, for example in the Google C++ style guide. – Maximilien

That's a good point. I would prefer not to expose internal storage to any other function - even in this style - since it limits you in modifying it in any case. I mostly wanted to ask him if he'd thought about that explanation. – Sabadilla