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?

Answer 1

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.

Answer 2

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.

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.

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;}
}

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())