6

There are a few different common patterns for returning the result of a function call in public APIs. It is not obvious which is the best approach. Is there a general consensus on a best practice, or, at least convincing reasons why one pattern is better the others?

Update By public API, I mean the public members that are exposed to dependent assemblies. I am not referring exclusively to an API that is exposed publicly as a web service. We can make the assumption that clients are using .NET.

I wrote a sample class below to illustrate the different patterns for returning values, and I have annotated them expressing my concerns for each one.

This is a bit of a long question, but I'm sure I'm not the only person to have considered this and hopefully this question will be interesting to others.

public class PublicApi<T>       //  I am using the class constraint on T, because 
    where T: class              //  I already understand that using out parameters
{                               //  on ValueTypes is discouraged (http://msdn.microsoft.com/en-us/library/ms182131.aspx)

    private readonly Func<object, bool> _validate;
    private readonly Func<object, T> _getMethod;

    public PublicApi(Func<object,bool> validate, Func<object,T> getMethod)
    {
        if(validate== null)
        {
            throw new ArgumentNullException("validate");
        }
        if(getMethod== null)
        {
            throw new ArgumentNullException("getMethod");
        }
        _validate = validate;
        _getMethod = getMethod;
    }

    //  This is the most intuitive signature, but it is unclear
    //  if the function worked as intended, so the caller has to
    //  validate that the function worked, which can complicates 
    //  the client's code, and possibly cause code repetition if 
    //  the validation occurs from within the API's method call.  
    //  It also may be unclear to the client whether or not this 
    //  method will cause exceptions.
    public T Get(object argument)
    {
        if(_validate(argument))
        {
            return _getMethod(argument);
        }
        throw new InvalidOperationException("Invalid argument.");
    }

    //  This fixes some of the problems in the previous method, but 
    //  introduces an out parameter, which can be controversial.
    //  It also seems to imply that the method will not every throw 
    //  an exception, and I'm not certain in what conditions that 
    //  implication is a good idea.
    public bool TryGet(object argument, out T entity)
    {
        if(_validate(argument))
        {
            entity = _getMethod(argument);
            return true;
        }
        entity = null;
        return false;
    }

    //  This is like the last one, but introduces a second out parameter to make
    //  any potential exceptions explicit.  
    public bool TryGet(object argument, out T entity, out Exception exception)
    {
        try
        {
            if (_validate(argument))
            {
                entity = _getMethod(argument);
                exception = null;
                return true;
            }
            entity = null;
            exception = null;   // It doesn't seem appropriate to throw an exception here
            return false;
        }
        catch(Exception ex)
        {
            entity = null;
            exception = ex;
            return false;
        }
    }

    //  The idea here is the same as the "bool TryGet(object argument, out T entity)" 
    //  method, but because of the Tuple class does not rely on an out parameter.
    public Tuple<T,bool> GetTuple(object argument)
    {
        //equivalent to:
        T entity;
        bool success = this.TryGet(argument, out entity);
        return Tuple.Create(entity, success);
    }

    //  The same as the last but with an explicit exception 
    public Tuple<T,bool,Exception> GetTupleWithException(object argument)
    {
        //equivalent to:
        T entity;
        Exception exception;
        bool success = this.TryGet(argument, out entity, out exception);
        return Tuple.Create(entity, success, exception);
    }

    //  A pattern I end up using is to have a generic result class
    //  My concern is that this may be "over-engineering" a simple
    //  method call.  I put the interface and sample implementation below  
    public IResult<T> GetResult(object argument)
    {
        //equivalent to:
        var tuple = this.GetTupleWithException(argument);
        return new ApiResult<T>(tuple.Item1, tuple.Item2, tuple.Item3);
    }
}

//  the result interface
public interface IResult<T>
{

    bool Success { get; }

    T ReturnValue { get; }

    Exception Exception { get; }

}

//  a sample result implementation
public class ApiResult<T> : IResult<T>
{
    private readonly bool _success;
    private readonly T _returnValue;
    private readonly Exception _exception;

    public ApiResult(T returnValue, bool success, Exception exception)
    {
        _returnValue = returnValue;
        _success = success;
        _exception = exception;
    }

    public bool Success
    {
        get { return _success; }
    }

    public T ReturnValue
    {
        get { return _returnValue; }
    }

    public Exception Exception
    {
        get { return _exception; }
    }
}
smartcaveman
  • 41,281
  • 29
  • 127
  • 212
  • any final solution with full source code ? – Kiquenet Aug 30 '12 at 10:16
  • @Kiquenet, I like to use a `Result` class similar to the one in the code sample, but with custom equality operators / overrides and two way implicit cast operators for ease of use. – smartcaveman Aug 30 '12 at 23:13

3 Answers3

6
  • Get - use this if validation failing is unexpected or if it's feasible for callers to validate the argument themselves before calling the method.

  • TryGet - use this if validation failing is expected. The TryXXX pattern can be assumed to be familiar due it's common use in the .NET Framework (e.g., Int32.TryParse or Dictonary<TKey, TValue>.TryGetValue).

  • TryGet with out Exception - an exception likely indicates a bug in the code passed as delegates to the class, because if the argument was invalid then _validate would return false instead of throwing an exception and _getMethod would not be called.

  • GetTuple, GetTupleWithException - never seen these before. I wouldn't recommend them, because a Tuple isn't self-explaining and thus not a good choice for a public interface.

  • GetResult - use this if _validate needs to return more information than a simple bool. I wouldn't use it to wrap exceptions (see: TryGet with out Exception).

dtb
  • 213,145
  • 36
  • 401
  • 431
  • 3
    I'd also add that there is a standard way to document thrown exceptions using inline XML documentation. These exceptions show up in Intellisense and will also be used when exporting help file documentation, using a tool like Sandcastle. As such, I highly recommend _not_ using the `out Exception` model (this generally destroys stack trace, too, if you want to rethrow.) – Dan Bryant Jul 21 '11 at 20:09
  • About the exceptions. What if the case is that this library is calling to a external source, and that call can sometiems fail (network down). Yet you dont want it to kill the entire application. Wouldnt it be valid to have a class that returns that the operation was not successful and as additional information has the exception? – Dzyann Mar 13 '13 at 15:36
  • Regarding the `out Exception` destroying stack trace: if the caller is prepared to parse nested exceptions then it might be okay. However, the typical C# code usually do not parse the nested exception. (NUnit is an example where nested exception is parsed.) – rwong May 02 '13 at 02:27
1

If by "public API" you mean an API by will be consumed by applications outside of your control and those client apps will written in a variety of languages/platforms I would suggest returning very basic types (e.g. strings, integers, decimals) and use something like JSON for more complex types.

I don't think you can expose a generic class in a public API since you don't know if the client will support generics.

Pattern-wise I would lean towards a REST-like approach rather than SOAP. Martin Fowler has a good article high level article on what this means: http://martinfowler.com/articles/richardsonMaturityModel.html

Hector Correa
  • 26,290
  • 8
  • 57
  • 73
-2

Things to consider, before answer:

1- There is a special situation about DOTNet programming languages & Java, due that you can easily retrieve objects, instead of only primitive types. Example: so a "plain C" A.P.I. may differ to a C# A.P.I.

2- If there is an error in you A.P.I., while retriving a data, how to handle, without interrumpting you application.

Answer:

A pattern, I have seen in several libraries, its a function, that its main result its an integer, in which 0 means "success", and another integer value means an specific error code.

The function may have several arguments, mostly read-only or input parameters, and a single reference or out parameter that maybe a primitive type a reference to an object, that maybe changed, or a pointer to an object or data structure.

In case of exceptions, some developers, may catch them and generate an specific error code.

public static class MyClass
{

  // not recomended:
  int static GetParams(ref thisObject, object Param1, object Params, object Param99)
  {
    const int ERROR_NONE = 0;

    try
    {
      ...
    }
    catch (System.DivideByZeroException dbz)
    {
      ERROR_NONE = ...;
      return ERROR_NONE;
    }
    catch (AnotherException dbz)
    {
      ERROR_NONE = ...;
      return ERROR_NONE;
    }

    return ERROR_NONE;
  } // method

  // recomended:
  int static Get(ref thisObject, object ParamsGroup)
  {
    const int ERROR_NONE = 0;


    try
    {
      ...
    }
    catch (System.DivideByZeroException dbz)
    {
      ERROR_NONE = ...;
      return ERROR_NONE;
    }
    catch (AnotherException dbz)
    {
      ErrorCode = ...;
      return ERROR_NONE;
    }

    return ERROR_NONE;
  } // method

} // class

Its similar to your tuple result. Cheers.

UPDATE 1: Mention about exception handling.

UPDATE 2: explicit declare constants.

umlcat
  • 4,091
  • 3
  • 19
  • 29
  • 1
    Error codes are usually not recommended in languages that feature exceptions. – dtb Jul 21 '11 at 20:04
  • @dtb I forgot: In libraries, similar to "assemblies", shared objects, dynamic libraries, exceptions may be catched and replaced byan error code. This technique depends on the developer. – umlcat Jul 21 '11 at 20:10
  • Well, the question is about best practices in C#. Error codes are explicitly *not* a best practice there. (Source: Framework Design Guidelines) – dtb Jul 21 '11 at 20:14
  • 1
    The biggest challenge with error codes is that they create a process flow that is 'assume the best'. If the developer omits to check the codes (and this invariably happens), execution continues, even though a previous step has failed. This usually creates a cascade of failures until finally something breaks badly enough that the software notices. Exceptions are an 'assume the worst' model, where you have to explicitly clarify that a particular failure case is acceptable. This causes code which was written without the errors in mind to terminate as early as possible. – Dan Bryant Jul 21 '11 at 20:15
  • What i wonder about this approach is, would it be considered OOP? It doesn't seem to me. It is not really self-explanatory. And in some cases you may be grabbing a group of exceptions in just one error code and therefore the person using the library could be missing very important information. I think that creating an actual object to return the condition is better. You could even have a hierarchy of classes for different errors, that way the person could use polimorphism to handle the exception. – Dzyann Mar 13 '13 at 15:31
  • @Dan Bryant. Your answer its a good example of "constructive critism". I use both Exceptions & return codes in O.O.P., but, it also, depends on the logic or flow of the application. – umlcat Mar 15 '13 at 17:48
  • @Dziann, I agree that error codes, its more procedural progr., than Object Oriented. But, after working, with several A.P.I., specially, Shared Libraries, there are some cases where error codes seems to be better tool. I work with O.O., most of the time, but, there are cases, where using functional, procedural, or other paradigms, are better suited. Cheers. – umlcat Mar 15 '13 at 17:53
  • -1 for giving bad advice (IMHO). @umlcat: Besides the fact that error codes really aren't the best idea in the context of .NET, yet another issue shows up in your example code: You are using literal numeric values (`ErrorCode = 0;`) rather than constants (e.g. `const int ERROR_NONE = 0; … ErrorCode = ERROR_NONE;`). The latter are much more self-documenting, have better IntelliSense / code completion support (in IDEs having that feature), and are easier to remember. – stakx - no longer contributing May 28 '13 at 08:21