is it better practice to return a complex object or use reference/out parameters?

Question

I'm putting together a method which is supposed to evaluate the input and return true if all conditions are met or false if some test fails. I'd also like to have some sort of status message available to the caller if there's a failure.

Designs I've come across include returning the bool and using an out (or ref) parameter for the message, returning an instance of a (specifically designed) class with the bool and string properties, or even returning an enum indicating pass or a specific error. what's the best way to get all the information out of the method? Are any of these "good"? Does anyone have other recommendations?

Answer 1

I prefer an object to out parms mainly because you can do more checking for valid values in the "Set" for the properties.

This is often overlooked in web apps, and a basic security measure.

OWASP guidelines state that values should be validated (using whitelist validation) every time they are assigned. If you're going to use these values in more than one place, it's far easier to create an object or a struct to hold the values, and check there, rather than checking every time you have an out parm in code.

Answer 2

I guess it depends on your definition of good.

For me, I'd prefer returning the bool with the out parameter. I think it's more readable. Seoncd to that (and better in some instances) is the Enum. As a personal choice, I tend not to like returning a class just for message data; it abstracts what I'm doing just one level too far away for my tastes.

Answer 3

I prefer directly returning the type as some .NET languages may not support ref and out parameters.

Here is a good explanation from MS as to why.

Answer 4

This type of thing is easily represented by Int32.Parse() and Int32.TryParse(). To return a status if it fails, or a value if it doesn't requires you to be able to return two different types, hence the out parameter in TryParse(). Returning a specialized object (in my opinion) just clutters your namespace with unnecessary types. Of course, you could always thrown an exception with your message inside it too. Personally, I prefer the TryParse() method because you can check a status without having to generate an exception, which is rather heavy.

Answer 5

This may be subjective.

But as always, I'd say that it pretty much depends and that there is not one "right" way to do it. For instance, the TryGet() pattern used by dictionaries returns a bool (which is often consumed in a if right away) and the effective return type as out. This makes perfect sense in that scenario.

However, if you enumerate the items you get KeyValuePair<,> - which also makes sense since you may need both the key and the value as one "package".

In your specific case, I may be tempted to actually expect a bool as result and pass in an optional (null allowed) ICollection<ErrorMessage> instance which receives the errors (ErrorMessage may just be String as well if that's enough). This has the benefit of allowing multiple errors to be reported.

Answer 6

I usually try to return a complex object and fall back to using an out parameter when I have to.

But you look at the TryParse methods in .NET's conversions, and they follow the pattern of returning a bool and an out parameter of the converted value. So, I don't think it's bad to have out parameters - it really depends on what you're trying to do.

Answer 7

Returning objects is more readable and takes less code. There are no performance differences, except for the your own while you're jumping through the hoops "out parameters" require.

Answer 8

I use enums, but use them as a flag/bit pattern. That way I can indicate multiple failed conditions.

[Flags]
enum FailState
{
    OK = 0x1; //I do this instead of 0 so that I can differentiate between
              // an enumeration that hasn't been set to anything 
              //and a true OK status.
    FailMode1 = 0x2;
    FailMode2 = 0x4;
}

So then in my method I just do this

FailState fail;

if (failCondition1)
{
    fail |= FailState.FailMode1;
}
if (failCondition2)
{
    fail |= FailState.FailMode2;
}

if (fail == 0x0)
{
    fail = FailState.OK;
}

return fail;

The annoying thing about this approach is that determining if a bit is set in the enumeration looks like this

if (FailState.FailMode1 == (FailState.FailMode1 && fail))
{
    //we know that we failed with FailMode1;
}

I use extension methods to give myself an IsFlagSet() method on my enumerations.

public static class EnumExtensions
{
    private static void CheckIsEnum<T>(bool withFlags)
    {
        if (!typeof(T).IsEnum)
            throw new ArgumentException(string.Format("Type '{0}' is not an enum", typeof(T).FullName));
        if (withFlags && !Attribute.IsDefined(typeof(T), typeof(FlagsAttribute)))
            throw new ArgumentException(string.Format("Type '{0}' doesn't have the 'Flags' attribute", typeof(T).FullName));
    }

    public static bool IsFlagSet<T>(this T value, T flag) where T : struct
    {
        CheckIsEnum<T>(true);
        long lValue = Convert.ToInt64(value);
        long lFlag = Convert.ToInt64(flag);
        return (lValue & lFlag) != 0;
    }
}

I copied this code from another answer here on SO, but I have no idea where that answer is at this point...

This allows me to just say

if (fail.IsFlagSet(FailState.Ok))
{
    //we're ok
}

Answer 9

I would strongly recommend returning a object (complex or otherwise), because undoubtedly you may find out in the future that you need to return additional information and if you used a combination of a simple return type with an out reference or two, you will be stuck into adding additional out parameters which will unnecessary clutter up your method signature.

If you use an object you can easily modify it down the road to support additional information that you need.

I would stay away from enumerations unless it is VERY simplistic and will not very likely change in the future. In the long run you'll have less headaches and things will be simpler if you return an object. The argument that it'll clutter the namespace is a weak one if you give return 'objects' their own namespace, but to each their own as well.

(Note if you want to use an enumeration, simply put that inside your returning object, this will give you the simplicity of enumerators combined with the flexibility of an object capable of whatever you need.)

Answer 10

EDIT: I'm adding a summary of my point at the top, for easier reading:

• If your method is returning multiple pieces of data that are logically all part of the same "thing", then definitely compose them into a complex object regardless of whether you are returning that object as the return value or an output parameter.

• If your method needs to return some kind of status (success/failure/how many records updated/etc.) in addition to data, then consider returning your data as an output parameter and using the return value to return the status

There are two variations of the situations to which this question applies:

1. Needing to return data that consists of multiple attributes

2. Needing to return data along with a status of the action used to obtain that data

For #1, my opinion is that if you have data consisting of multiple attributes that all go together, then they should be composed into a single type as a class or struct, and a single object should be returned as either the return value of a method or as an output parameter.

For #2, I think this is the case where output parameters really make sense. Conceptually, methods typically perform an action; in this case, I prefer to use the return value of the method to indicate the status of the action. That could be a simple boolean to indicate success or failure, or it could be something more complicated such as an enum or string if there are multiple possible states to describe the action that the method performed.

If using output parameters I would encourage a person to use only one (refer to point #1), unless there is a specific reason to use more than one. Do not use multiple output parameters simply because the data you need to return consists of multiple attributes. Only use multiple output parameters if the semantics of your method specifically dictate it. Below is an example where I think multiple output parameters make sense:

    // an acceptable use of multiple output parameters
    bool DetermineWinners(IEnumerable<Player> players, out Player first, out Player second, out Player third)
    {
        // ...
    }

Conversely, here is an example where I think multiple output parameters do not make sense.

    // Baaaaad
    bool FindPerson(string firstName, string lastName, out int personId, out string address, out string phoneNumber)
    {
        // ...
    }

The attributes of the data (personId, address, and phoneNumber) should be part of a Person object returned by the method. A better version of it would be the following:

    // better
    bool FindPerson(string firstName, string lastName, out Person person)
    {
        // ...
    }

Answer 11

Your method should throw an exception if any of the test conditions fail, that can contain the status message. That way you won't have output parameters and you won't return a big object.

Answer 12

The TryParse(...) method is not the best example in this case. TryParse only signals if the number was parsed or not, but it does not specify why the parsing failed. It doesn't need to because the only reason it can fail is due to "bad format".

When you come up with situations where there is no obvious good method signature for what your are trying to do you should step back and ask yourself if the method is maybe doing too much.

What tests are you running? Do they relate at all? Are there tests in this same method that would fail due to completely different unrelated reasons that you have to track in order to give meaningful feedback to the user/consumer?

Would refactoring your code help? Can you group tests logically into seperate methods that can only fail due to one reason (single purpose principle) and call them all accordingly from your original callsite instead of the one multipurpose method you are considering?

Is a failed test a normal situation that only needs to be logged to some log file or is it mandatory that you inform the user in some inmeadiate way probably halting the normal flow of the application? If its the latter, would it be possible to use custom exceptions and catch them accordingly?

There are many possibilities but we need more information about what you are trying to do in order to give you more precise advice.

Answer 13

We do something very similar with our WCF services using Request and Response objects, and exceptions are bubbled all the way up (not handled in the method itself) and handled by a custom attribute (written in PostSharp) which catches the exception and returns a 'failure' response, e.g.:

[HandleException]
public Response DoSomething(Request request)
{
   ...

   return new Response { Success = true };
}

public class Response
{
   ...
   public bool Success { get; set; }
   public string StatusMessage { get; set; }
   public int? ErrorCode { get; set; }
}

public class Request
{
   ...
}

The exception handling logic in the attribute can use the Exception message to set the StatusMessage and if you have a set of custom exceptions in your app you can even set the ErrorCode where possible.

Answer 14

I think it depends on the level of complexity, and whether the operation could fail, resulting in...what?

In the case of the TryParse examples, you typically cannot return an invalid value (e.g. there is no such thing as an "invalid" int) and definitely cannot return a null value type (the corresponding Parse methods don't return Nullable<T>). Additionally, returning Nullable<T> would require introduction of a Nullable<T> variable at the call site, a conditional check to see if the value is null, followed by a cast to the T type, which has to duplicate the runtime check to see if the value is null before returning that actual non-null value. This is less efficient that passing the result as a out parameter and indicating via return value success or failure (obviously, in this case, you're not concerned why it failed, only that it did). In prior .Net runtimes, the only choice was the Parse methods, that threw on any sort of failure. Catching .Net exceptions is expensive, especially relative to returning a true/false flag indicating status.

In terms of your own interface design, you need to consider why you would need an out parameter. Maybe you need to return multiple values from a function and cannot use anonymous types. Maybe you've need to return a value type, but cannot produce a suitable default value according to inputs.

out and ref parameters are not no-nos, but your interface should be carefully considered as far as whether they are necessary or not, before you use them. A standard return by reference should be sufficient in more than 99% of cases, but I would look at using out or ref parameters before unnecessarily defining a new type just to satisfy returning a "single" object instead of multiple values.

Answer 15

"I'm putting together a method which is supposed to evaluate the input and return true if all conditions are met or false if some test fails. I'd also like to have some sort of status message available to the caller if there's a failure."

If your method has conditions with regard to input and if they are not met, you can very much consider throwing appropriate exceptions. By throwing proper exceptions the caller will know what exactly went wrong. This is more maintainble and scalable approach. Returning error code is not a good options for obvious reasons.

"Designs I've come across include returning the bool and using an out (or ref) parameter for the message, returning an instance of a (specifically designed) class with the bool and string properties, or even returning an enum indicating pass or a specific error. "

1. Complex objects are ofcourse most obvious option. (class or structure)

2. Out parameter: "One way to think of out parameters is that they are like additional return values of a method. They are very convenient when a method returns more than one value, in this example firstName and lastName. Out parameters can be abused however. As a matter of good programming style if you find yourself writing a method with many out parameters then you should think about refactoring your code. One possible solution is to package all the return values into a single struct." Too many out or ref parameter points to a design flaw.

3. Tuple: "group together a bunch of otherwise unrelated data in some structure that is more lightweight than a class"

Must read: More on tuples: