3

I just have a dilemma, how should I return smart pointer from a function, when function might fail. I could pick one of the following options:

  1. Return pointer, and throw exception if function fails:

    std::shared_ptr foo() {
        // ...
        if (!ok)
          throw;
        return ptr;
    }
  1. Return pointer, and return empty pointer, if function fails

    std::shared_ptr foo() {
        // ...
        if (!ok)
            return std::shared_ptr();

        return ptr;
    }
  1. Pass pointer by reference, and return bool flag

    bool foo(std::shared_ptr& ptr) {
        // ...
        if (ok)
            ptr = ...;

        return ok;
    }

Is there any best practice, guideline, how to report, that function didn't execute properly? Or is it usually project-specific?

Thanks for your answers

Marcin Kolny
  • 633
  • 7
  • 14
  • 1
    Very related: [Should a retrieval method return 'null' or throw an exception when it can't produce the return value?](http://stackoverflow.com/q/175532/3425536) – Emil Laine Apr 21 '16 at 22:26
  • 1
    Any answers are going to be highly opinion based. However, regardless of whether someone agrees with your decision **just make sure the behavior is documented**. – James Adkison Apr 21 '16 at 22:26
  • 2
    Don't use #3, #1 is better; it doesn't let the caller forget to handle errors. #2 is fine because the caller has to check for null anyway. And as usual, none of these are hard rules because sometimes your environment imposes additional constraints (exceptions too slow/don't exist, etc.) – GManNickG Apr 21 '16 at 22:28
  • 1
    Definitely not #3, out parameters are terrible. #1 needs to actually throw something, other your program will terminate if you `throw;` without an active exception. #1 and #2 are both OK, but some codebases disallow exceptions, in which case #2 is your only option. My preference would be to use #2 unless this is a case where it's unlikely that the caller can do something sensible if `foo()` fails, in which case you should go with #1. – Praetorian Apr 21 '16 at 22:30
  • Since smart pointers can be null, if this is not the right time to utilize that feature, what is? – Emil Laine Apr 21 '16 at 22:30
  • considering `shared_ptr`, option `2` is better due to return type being able to have null value. For return types that don't have invalid value, I prefer functions to be declared as `return_type function(bool *ok = nullptr)`, where `ok` is only set if not pointed by nullptr. It is especially useful if you know in some specific cases your function never fails. – Andrei R. Apr 22 '16 at 04:54
  • Perhaps this is just an opinion, but perhaps the problem is that the function has more than one responsibility. The act of doing something should be separate to the act of providing the pointer. The one may act as pre-condition for the other, though. One could return a proxy (which is what boost::optional provides) that returns based on. The subsequent getting of the pointer is a Post-Op. – Werner Erasmus Apr 22 '16 at 10:03

2 Answers2

1

Honestly the correct answer depends on what the called function does, and what the consequence of the failure is. For libraries I'd suggest either throwing an exception or returning a value that indicates failure. Passing the pointer by reference and returning a flag seems questionable unless you're using that idiom frequently, or if there is a reason to externally manage the shared pointer.

John Percival Hackworth
  • 11,395
  • 2
  • 29
  • 38
-1

Good question. I think it depends on the occurrence or nature of the error. If you foresee this error to happen semi-occasionally or it's an expected error, or even it's an error that can be recovered somehow then I would use method 2 or 3.

However, if the error is something that shouldn't normally happen and it would cause the application to not function properly I would use the 1st method.

My reasoning for this is that exceptions could possibly be slower compared to the other two methods (see Are Exceptions in C++ really slow, and by nature exceptions flow up to the main caller so it natural that you want a critical error to move up and stop the process.

Community
  • 1
  • 1
ArmenB
  • 2,125
  • 3
  • 23
  • 47