Named Output Parameters vs Return Values – Best Practices

documentationfunctionsout-parameters

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?

Best Answer

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.

Related Topic