Named output parameters vs return values

Question

Which code is better:

// C++
void handle_message(...some input parameters..., bool& wasHandled)
void set_some_value(int newValue, int* oldValue = nullptr) 
// C#
void handle_message(...some input parameters..., out bool wasHandled)
void set_some_value(int newValue, out int oldValue)

or

bool handle_message(...some input parameters...) ///< Returns -1 if message was handled
 //(sorry, this documentation was broken a year ago and we're too busy to fix it)
int set_some_value(T newValue) // (well, it's obvious what this function returns, so I didn't write any documentation for it)

The first one doesn't have any documentation, but it doesn't need it. It's a self-documenting code. Output value clearly says what it means, and it's really hard to make a change like this:

- void handle_message(Message msg, bool& wasHandled) {
- wasHandled = false;
- if (...) { wasHandled = true; ...
+ void handle_message(Message msg, int& wasHandled) {
+ wasHandled = -1;
+ if (...) { wasHandled = ...;

When the code uses return values, it's easy to change them and leave the comments broken:

 /// Return true if message was handled
- bool handle_message(Message msg) {
+ int handle_message(Message msg) {
...
- return true;
+ return -1;

Most of compilers don't (and can't) check the documentation which is written in comments. Programmers also tend to ignore comments when they edit the code.

So the question is:
if a subroutine has a single output value,
should it be a procedure with well-named self-documenting output parameter,
or should it be a function which returns an unnamed value with a comment describing it?

• unholysampler: Bad documentation will always cause issues. It is not something special that out parameters fix.
• Richard: Well, now that edit changes things. :P
• Bobson: @Richard - No, the OP's question is "No return and well-named output or unnamed result with comments." As I said, it should be a well named function with result, which is neither of those two, although it's closer to the latter.
• Bobson: @Abyx - That depends on what it's supposed to do. bool is_valid_message(Message msg). bool try_handling_message(Message msg). int get_message_status(Message msg). MessageResult process_message(Message msg).... The first is clearly true if it's a valid message. The second is (by convention) true if the message was handled successfully. The third is clearly some type of status value (which you'd have to look up regardless if it was from a return or an out). The last is very clear about what it is.
• JustinC: Side-effect free functions are hard to write if you are relying on output tagged parameters.

Answer 1

Edit (since the question has changed)

If your method only has one output, of course you return that value.

bool DoStuff(...)

is much more readable when you use it than

void DoStuff(..., out bool success)

Look at the sample usage:

if(DoStuff(....))

vs

DoStuff(..., out success)
if (success)

Also, it allows in-lining and chaining (if the language supports it):

ProcessRequest(GetRequest(reqID))
newval = ProcessString(oldval).Replace("/t","")

Converting these to "out" params would lead to much uglier code, plus it's just wrong.

For 1 return, always use return values.

• Abyx: When I see bool DoStuff(...) I have no idea what that bool means. Is it "that stuff was good, let's do it again" or "we did enough of that stuff, let's stop" or something else - I don't know. Or maybe it has nothing to do with doing stuff? But with void DoStuff(..., out bool success) or void DoStuff(..., out bool dontDoItAgain) I know what that output value means.
• Bobson: @Abyx - That's a problem with the function name being DoStuff(), not with needing a return value.
• Richard: Yeah, if you want to return a failure condition from a function, you need to name it appropriately, so that it's clear what the return value is. DoStuff() returning a boolean would be "true" for success. FailToDoStuff() would return "true" for failure--but that's an entirely different pattern.
• Richard: Another example: if ProcessRequest() returns a boolean, "true" should always mean "success". Otherwise, it's just nonsensical.
• JustinC: ProcessRequest returning true isn't a great example. Even if processing failed, it still processed as much as it could for as long as it could. true || false is ambiguous. Are you trying to derive a conclusion from the input? isActionable Or are you attempting to transform and record it? wasRecorded And so on..

Answer 2

Make functions return values. This isn't any sort of functional thing or even relatively modern. If you're unclear about what the function is returning, then your function needs a better name. If your code is doing the wrong things, fix and/or test it.

In a vacuum, the only time that modifying your inputs is acceptable is when that input is this in object oriented languages or when the return value is taken by someother part of the function call (things like TryParse).

edit: (because the question changed)

So, again, the question is: if subroutine has single output value, should it be a procedure with well-named self-documenting output parameter, or should it be a function which returns an unnamed value and have a comment describing it?

Neither. It should be a function that is well named so that its output is clear at the call-site. The comment at the declaration only helps in the declaration. Making comments at all the call-sites is inane and unmaintainable.

If you can't make a good function name to describe its output, rethink your design.

• Telastyn: @richard - Always (except for the two scenarios above). Frankly, distinguishing between functions, methods and procedures is fairly meaningless. They're all the same thing under the hood, and should follow the same design guidelines.
• Telastyn: @richard - sorry about the edit. I will use them for the Try<foo> cases, but will work to avoid them. Multiple outputs are to be avoided, as they're a code smell for violating Single Responsibility (which I find is just as applicable in function design).
• Andres F.: @Abyx In C++, void functions aren't functions. They are procedures by another name. C++ does have procedures, like you pointed out; it just doesn't single them out by name. So your question remains: "when should we prefer functions to procedures, and why?".
• Bobson: @Abyx - And the answer we're giving you is neither.
• Telastyn: @abyx - functions that do not return values necessarily have side effects (even if that side effect is on the environment), or else they're meaningless.

Answer 3

One reason to write proper functions with return values (as opposed to procedures-in-disguise with "output" variables) is composability.

If you have functions (excuse my pseudocode) void f(in: int x, out: int y) and int g(in: int x), how do you compose them?

You cannot apply g to f applied to... say, 42:

int y = g(f(42));

Instead, you need to write:

int x = 0;
f(42, x);
int y = g(x);

Which is definitely clumsier and more error-prone. And composability matters a lot in languages with first-class functions.

---

Here is another example, hopefully more convincing: I'd like to be able to write

int maxSum = max(sumOfArray(array1), sumOfArray(array2)));

instead of jumping through hoops to achieve the same:

int sum1 = 0;
sumOfArray(array1, sum1);
int sum2 = 0;
sumOfArray(array2, sum2);
int maxSum = 0;
max(sum1, sum2, maxSum);

Hopefully this is a less contrived scenario illustrating why function composition is useful, and why it requires that the function returns its result instead of modifying an "out" variable.

• jk.: @Abyx yes they will have better names in reality but totally disagree that you don't want to compose functions IRL
• Andres F.: @Abyx I thought it was clear f and g are placeholder names, and not actual function names. Same with variable names. You are arguing minutiae instead of the actual point. Was the downvote yours?
• Andres F.: @Abyx Haskell is used in real-world software, unlike brainfuck. Are you trolling? Your post was initially language agnostic; if you only care about C++ and C#, mark it as such.
• Andres F.: If you can't edit the source code, then you can't do anything about preexisting functions, neither adding an output variable nor renaming the function, so the point is moot. If you can edit the source code, then the right course of action is renaming the function and having it return the proper value.
• Andres F.: @Abyx I disagree about GHC. But if you prefer, here's Scala's map signature: def map[B](f: (A) ⇒ B): TraversableOnce[B] from Iterable. Notice the f in there? It means "any function with the right signature" ;)

Answer 4

You can make your function return something that is obviously a return value. It's also useful to send error message back to the UI when something fails.

ReturnValue myFunction(object newValue)
class ReturnValue
{
 public bool Success {get;set;}
 public object OldValue {get;set;}
 public string Message {get;set;}
}

• Abyx: Yes. But it often ends with MyFunctionReturnValueType, because you have a lot of different functions with different return types or different meanings of their return values.
• Ingo: Often, a Tuple<A,B> is sufficient.
• Abyx: @Ingo with tuple<string, string> you have no idea what output values are, or what is their order in tuple.
• Ingo: @Abyx - Some documentation is unavoidable in any case.

Answer 5

Your question is

If subroutine has single output value, should it be a procedure with well-named self-documenting output parameter, or should it be a function which returns an unnamed value and have a comment describing it?

and the answer to that is neither (or, perhaps, both). If you only have a single output value, you should definitely be using functions, not void procedures. But the function's name should make it trivial to see what the return value is, so it's not an "unnamed" value and it doesn't need a comment.

Your example is called handle_value(Message msg). This is a bad function name, unless you don't care about the results (although see the last example below). Compare it to C#'s default for handling events: void btnCreateTransfer_Click - the calling function doesn't care what happens inside this function, so we don't return anything, and the name just says that it does a "click" on "btnCreateTransfer".

Depending on what you want to get back out of your handle_value logic, there's a number of ways you can rename it.

• If you just want to check that it is able to be handled (i.e. the message was valid), you can define it as bool is_valid_message(Message msg). The result is clearly true if it is valid, and false if not.

  • C#: string.IsNullOrEmpty()

• If you just want to know whether it was successfully handled, you can name it bool try_handling_message(Message msg), or bool handled_message(Message msg), or have a coding convention that any bool result from an unclear function is expressly success or failure.

  • C#: int.TryParse() - this has an out parameter for the value, but the function itself tells you whether it was successful.

• If you need more details from your response, you can use int get_message_status(Message msg). Obviously, you'd need some way to know what the int result meant. It could have meaning on its own or it could be defined in a constant.

  • C#: Stream.Read() returns the number of bytes read.

• Finally, you can return a custom type - either an enum or an actual class. The enum works just like the int, except that it conveys meaning of its own so it gives you the most freedom of naming. The class lets you encapsulate and return as much data as you need.

  • C#: DialogResult myform.ShowDialog() - ShowDialog() isn't very explicit about what it returns, but because it's a DialogResult value, you know exactly what it is returning.
  • MessageResult process_message(Message msg) - You process the message and get the result of doing so. This can also be your original handle_message, but then it would be something like HandledMessageResult handle_message(Message msg).

Good code is self-documenting in that the functions, classes, and variables all explain what they are and what they do. If you can name an out parameter clearly enough to know what's being returned, you can work that into the name of the function. And if you can't name your out well and you can't name the function well, then you can still name the type of the return usefully. (see ShowDialog())

• Abyx: oh, WinForms events. Did you notice that all of them have void return type? Why do you ignore all the events which have output values? E.g. FormClosing, Drag&Drop events, etc?
• Abyx: in handle_message(out bool wasHandled), false value of wasHandled doesn't mean "failure" or "success". It means that there is no need to pass message to next handler. Something like "stop event propagation" if we'd use term "event" instead of "message". As for Windows window messages, DOM events, etc. And yeah, wasHandled has nothing to do with message result and actual message processing. Handler can process message, but return wasHandled == false if it wants to pass the message to a next handler.