2

I often use the pattern

bool TryGetX(out X x)

when I need to get data from a service that may fail. I use this when neither the client nor the service can know if the data is available until trying to retrieve it. I've never been totally happy with using the out parameter that way, and I see from other SO questions that it is considered a poor pattern.

However, I haven't seen a convincing alternative API that gives the client a hint that their method call may not actually return the data.

Alternatives I've considered:

  • X GetX(), where GetX() returns null if the service cannot provide the requested data. I don't like doing this since it isn't clear to many people that they need to check for null being returned. Given the number of null reference exceptions I encounter in legacy code I've worked on, people often neglect a null-check and the user gets an ugly exception box shown to them. At least the TryGetX approach is clear to the client that the data may not be returned. Even if they do check for null, is requiring a if (x == null) everywhere really better than TryGetX(out X x)?

  • Try/Catch, where GetX() throws an exception if the data cannot be returned. Again, clients may not realize that GetX() throws an exception, and even if they do, you have Try/Catches sprinkled around the code.

  • GetXResult GetX(), where the class GetXResult has two properties: bool Success and X x. This would probably be my preferred alternative, but then I have many result classes floating around, cluttering my project.

What is the best approach for a method that returns data, but may not be able to return the data requested?

GreenRibbon
  • 299
  • 2
  • 16
  • 2
    Create a `Maybe` type which encodes the possible non-existence of a value and return that. – Lee Jan 15 '16 at 17:12
  • Have you considered just using `X TryGetX()` where it *may* return `null` if the attempt failed? If `X` can be a value type that may not be a good idea though. – Lasse V. Karlsen Jan 15 '16 at 17:13
  • @LasseV.Karlsen How is that different than the first option listed? – Hogan Jan 15 '16 at 17:15
  • I'd probably favor the Try/Catch pattern. At least if the user doesn't realize that the call can fail, at least the exception will be thrown right from the place where the problem occurred. – Matt Burland Jan 15 '16 at 17:15
  • @Hogan It is clearer, at least to me, that the attempt may fail and thus not return an object. – Lasse V. Karlsen Jan 15 '16 at 17:16
  • @LasseV.Karlsen The OP says "returns null if the service cannot provide the requested data." Isn't that the same as "where it may return null if the attempt failed"?? – Hogan Jan 15 '16 at 17:19
  • there is actually a section dedicated to this very issue in the official microsoft dot net frameworkk guidelines. –  Jan 15 '16 at 17:59
  • You don't mention the big benefit of case 2 which is the exception can contain a lot more detail describing the problem. Also case 3 suffers from the same problem as the rest, a programmer forgetting to check for success or failure. I don't think there's any way around that. – Mike Zboray Jan 15 '16 at 18:20

1 Answers1

4

Best approach depends on the problem, I doubt that there is a 'best' approach that fits all the problems.

X GetX(), where GetX() returns null if the service cannot provide the requested data.

If all of the types that the service needs to return are Nullable and there can be kind of consistency with all these type of methods, this is not a bad approach at all. In order to clarify the 'null' return to clients, you may include 'Try' in method name (documentation also helps clients).

Try/Catch, where GetX() throws an exception if the data cannot be returned.

Throwing or not throwing exceptions is a decision that should be made in service architecture. If you choose to throw exceptions in that level for all your methods that need to, this approach also works fine. (use documentation to inform clients).

GetXResult GetX(), where the class GetXResult has two properties: bool Success and X x.

In orderto solve the 'too many classes' problem simply use generics:

sealed class Result<T> {
    private bool success = false;
    private T value;
    private Result() {
    }
    public Result(T value) {
       this.success = true;
       this.value = value;
    }
    public static Result<T> Failure() { 
       return new Result<T>();
    } 
    public bool Success {
       get {
          return this.success;
       }
    }
    public T Value {
       get {
          if (!this.success) {
             throw new InvalidOperationException();
          }
          return this.value;

       }
    }
}
Mehrzad Chehraz
  • 5,092
  • 2
  • 17
  • 28
  • Could you add something about why using an _out_ parameter is bad? Or maybe you think it isn't? – GreenRibbon Jan 15 '16 at 18:18
  • @GreenRibbon I don't think it is bad in all circumstances, the OP wants another approach. Anyway you can read https://msdn.microsoft.com/en-us/library/ms182131.aspx and also http://stackoverflow.com/questions/2366741/what-is-bad-practice-when-using-out-parameters. – Mehrzad Chehraz Jan 15 '16 at 18:22
  • Thanks for those links. The MSDN page lists _out_ as an "intermediate" skill. If so, I'd expect anyone working on my code-base to be of "intermediate" skill, or at least using _out_ as an opportunity to build their skill when they first encounter it. – GreenRibbon Jan 15 '16 at 18:28
  • @GreenRibbon, Well you may do so, but you should consider that skill is not the only problem with out parameters. In the other hand it is not a "MUST NOT USE" feature of the language IMO but I would avoid it whenever possible for the sake of simplicity and more clean code. – Mehrzad Chehraz Jan 15 '16 at 18:35