3

I've started making use of std::unique_ptr e.g.:

unique_ptr<TFile> myfile( TFile::Open("myfile.root") );

instead of 

TFile * myoldfile = TFile::Open("myoldfile.root") ;

Now I'm not sure what my functions should look like. I think part of the problem might be that my code is not complicated enough yet for any problems to become apparent but I'd like to get it right now so I don't end up in a mess when things are more complicated.

I used to have:

double interestingResult( TFile * input )
{...}

(which doesn't actually modify TFile but can't be const because it calls some non-const functions of TFile).

I could still call this by doing:

myResult  = interestingResult( myfile.get() );

which doesn't seem very user friendly. (I think it is suggested here: https://stackoverflow.com/a/5325560/1527126 .)

Or I could modify my function to look like:

double interestingResult( unique_ptr<TFile>& input )
{...}

which forces the user to always use a unique_ptr.

or I could support both by writing:

double interestingResult( unique_ptr<TFile>& input )
{ return interestingResult( intput.get() ); }

but I don't see that in other people's code.

What is the standard approach to this situation?

I think this answer (https://stackoverflow.com/a/9700189/1527126) implies I should accept references since I don't expect myfile to be null. But the library function TFile::Open always returns a pointer so it seems natural to be able to pass that straight into the function without extra dereferencing.

Apologetic p.s. I gave up trying to work out whether I should ask on StackOverflow or CodeReview or not at all but please point me to the appropriate place if this is not it.

Community
  • 1
  • 1
paco_uk
  • 445
  • 1
  • 5
  • 15

4 Answers4

4

The answer you are referring to is the right one.

If you have a function that should work on any existing TFile instance and has no reason to accept NULL pointers as a valid argument and where the function argument does not carry ownership, you should use TFile& as parameter type (TFile const& where possible). That the object being passed in is held by pointer (with or without ownership) by the caller should not make a difference to that function.

You could choose to use a TFile * argument, but that creates at least ambiguity over validity of passing NULL, so may cause problems in the future.

If you use a unique_ptr<TFile> argument, you pass ownership of the object into the function for good. The caller will be left with a NULL unique_ptr when the call returns.

If you use a unique_ptr<TFile>& argument, this indicates that the function can choose to take over ownership of the object or leave it to the caller. Rather unusual.

A unique_ptr<TFile> const& argument could be used like a TFile *, but forces the caller to own the object and to manage it using a unique_ptr. Why should such a requirement be imposed on the caller? Of course this (and the other uses of unique_ptr) all also have to deal with the NULL case.

JoergB
  • 4,383
  • 21
  • 19
3

The problem with accepting smart pointers in the parameter list is you dictate what kind of smart pointer the caller can use. For example, by using unique_ptr you prevent the caller from using a shared_ptr instead.

In your case, I'd suggest a reference parameter. I also see regular pointers used a lot as well. The main thing is that your function doesn't try to hold a reference / take ownership of the object after the function returns - i.e. it's not later responsible for freeing the memory.

James Johnston
  • 9,264
  • 9
  • 48
  • 76
2

Either a reference TFile & or a pointer TFile * is fine.

There are people who will tell you that you "should" use a reference when a null pointer is an invalid input to the function. These people become confused and angry when they try to use standard functions like std::strlen, std::memcpy etc, inherited from C. My feeling is that while it's quite nice to use references to self-document that a referand is required, it's also quite nice to have your API work either consistently with references or consistently with pointers.

There are people who will tell you that you "should not" use a non-const reference parameter, preferring a pointer. They become confused and angry when they try to use standard functions like std::swap, because to a C programmer that looks like pass-by-value and so it "should not" modify the input.

As long as you don't work with either of those people, you can make your own choice.

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
  • Use of a pointer type when NULL is invalid value regularly leads to maintenance problems. At best it makes use of NULL a runtime problem - requiring repeated checks and a conduit to signal the error case. Or else it causes crashes or worse, if there is a call path where a NULL can slip through. If its the style of the project or you run into fear of non-const references, that is what you have to do - but you should be aware of the consequences. FYI: I am not afraid of using `strcpy` in C(-style) code, but I'd use a string class instead of naked C strings when I get a choice. – JoergB Mar 13 '13 at 17:02
  • 1
    @JoergB: I haven't observed those regular maintenance problems being any worse in code that takes pointers. The kind of person who doesn't bother checking in the documentation whether `null` is a valid input (i.e. almost anyone, on a sufficiently bad day) also doesn't bother checking for null before dereferencing a pointer. So they just pass `*p` to your function that takes a reference, and sometimes `p` is null in their code, and someone has to find out about it and fix it. – Steve Jessop Mar 13 '13 at 17:03
  • 2
    And FWIW, I become confused and angry when I try to use functions that take a pointer, but don't say explicitly whether null inputs are valid because to the author it's obvious by convention that if the function takes a pointer then null is valid ;-) – Steve Jessop Mar 13 '13 at 17:09
  • This answer plus its comments seem to cover everything. My understanding is now: never accept smart pointers unless you want to take ownership. Ideally accept references. If everyone else is using pointers for similar functions it's probably better to be consistent. – paco_uk Mar 13 '13 at 19:53
1

It depends, but using raw pointers(or references to object) in functions prefered, because functions do not take ownership of pointer.

And also if one day you'll decide to use shared_ptr's...

kassak
  • 3,974
  • 1
  • 25
  • 36