Recommended way of returning pointer from C++ function?

I have a pointer that is passed to a series of functions, where one function returns an address and assigns that address to the pointer. After seeking some help on Stackoverflow, the collect_file_path method has a parameter of type QStringList**. Although I understand what is going on I have not come across this notation in any of the text books I own, plus, it looks ugly.

I would like some advice/feedback on how other programmers would implement what I have don开发者_开发技巧e in the code below. Would you use QStringList**, or some other method?

I hope what I'm asking makes sense. My current code is:

void ThreadWorker::run()
{
  QStringList* file_list;
  collect_file_paths(&file_list);
}

void ThreadWorker::collect_file_paths(QStringList** file_list)
{
  DirectorySearch ds;
  *file_list = ds.get_file_names(_strPath);
}

QStringList* DirectorySearch::get_file_names(QString path)
{
  QStringList *file_names = new QStringList;
  traverse(path, file_names);
  return file_names;
}

Thanks

The problem with returning a bare pointer is that it's quite easy to end up leaking the memory for the object. This might happen for example if anything in your function throws; but it can also happen on the caller's side, before they have received the pointer (e.g. if it is used as an argument to a function, and the evaluation of another argument throws).

For these reasons, it's best to wrap pointers in a smart pointer like std::auto_ptr (unique_ptr in the upcoming C++ standard) or boost::shared_ptr (also available as std::shared_ptr, soon in a standard library near you). These wrappers will safely take care of deleting the wrapped pointer when the time comes to do so.

However, in this specific case you can manage without using a QStringList** by passing the QStringList* as a reference.

You can pass by pointer reference also, which is more C++ style:

void ThreadWorker::collect_file_paths(QStringList*& file_list) <-- see *&
{
}

You don't have to pass the address of file_list now:

collect_file_paths(file_list);  // simply pass it

But instead of that, I would still recommend following approach (which is simpler):

void ThreadWorker::run()
{
  QStringList* file_list = collect_file_paths();
}

QStringList* ThreadWorker::collect_file_paths()
{
  DirectorySearch ds;  //<---- ds uninitialized
  return ds.get_file_names(_strPath);  // for static function use ClassName::method() style
}

Just return by value. The compiler can RVO, NRVO and you can swaptimize.

You can return by value, since unnecessary copies will be optimized away. This is a clear, exception safe and my recommended way.

void ThreadWorker::run()
{
  QStringList file_list = collect_file_paths();
}

QStringList ThreadWorker::collect_file_paths()
{
  DirectorySearch ds;
  return ds.get_file_names(strPath_);  // you should not use leading underscore in c++
}

QStringList DirectorySearch::get_file_names(QString path)
{
  QStringList file_names;
  traverse(path, &file_names);
  return file_names;
}

http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/

Btw. I think Qt Containers have copy on write optimizations, so the copy will be cheap even without compiler optimizations.

If you don't trust in (very common) compiler optimizations like RVO (comment by heishe) and you do not use copy-on-write objects like the QStringList you should create the instance in the run() method and pass a reference to the other functions. This is not as clear as my recommended technique but it is still exception safe (at least the basic guarantee).

void ThreadWorker::run()
{
  QStringList file_list;
  collect_file_paths(file_list);
}

void ThreadWorker::collect_file_paths(QStringList& file_list)
{
  DirectorySearch ds;
  ds.get_file_names(strPath_, file_list);  
}

void DirectorySearch::get_file_names(QString path, QStringList& file_list)
{
  traverse(path, &file_list);
}

A 3rd solution is to return a smart pointer like std::unique_ptr<QStringList>. But I cannot see any reason for an additional dynamic allocation in this example.

It might look ugly but it's not that uncommon in some APIs to pass a double pointer.

In your case it seems unnecessary. Why does collect_file_paths not just return a pointer too?

If you have allocated the memory with new-operator, you can just return the pointer. Also, remember to use delete to the allocated memory. Good place for this would normally be in the destructor method (not in this case, since you only use the memory in the run-method).

void ThreadWorker::run()
{
  QStringList* file_list;
  file_list = ds.get_file_names(_strPath);

  //here we do something with file_list
  //...

  //Free the memory. You have to do this if the object pointed by file_list is not
  //used anywhere else.
  delete file_list;
}

QStringList* DirectorySearch::get_file_names(QString path)
{
  QStringList *file_names = new QStringList;
  traverse(path, file_names);
  return file_names;
}

Assuming that the user has access to the header but not to the implementation, he/she hasn't got any idea how to handle the pointer. If your function is a source (aka allocate the pointer with new) you should return an auto_ptr. auto_ptr are: standard, never throws and specifically design to do this job.

You may have a look at this http://www.gotw.ca/publications/using_auto_ptr_effectively.htm