30

I occasionally run into methods where a developer chose to return something which isn't critical to the function. I mean, when looking at the code, it apparently works just as nice as a void and after a moment of thought, I ask "Why?" Does this sound familiar?

Sometimes I would agree that most often it is better to return something like a bool or int, rather then just do a void. I'm not sure though, in the big picture, about the pros and cons.

Depending on situation, returning an int can make the caller aware of the amount of rows or objects affected by the method (e.g., 5 records saved to MSSQL). If a method like "InsertSomething" returns a boolean, I can have the method designed to return true if success, else false. The caller can choose to act or not on that information.

On the other hand,

  • May it lead to a less clear purpose of a method call? Bad coding often forces me to double-check the method content. If it returns something, it tells you that the method is of a type you have to do something with the returned result.
  • Another issue would be, if the method implementation is unknown to you, what did the developer decide to return that isn't function critical? Of course you can comment it.
  • The return value has to be processed, when the processing could be ended at the closing bracket of method.
  • What happens under the hood? Did the called method get false because of a thrown error? Or did it return false due to the evaluated result?

What are your experiences with this? How would you act on this?

christophos
  • 35
Independent
  • 830

5 Answers5

34

In the case of a bool return value to indicate the success or failure of a method, I prefer the Try-prefix paradigm used in various in .NET methods.

For example a void InsertRow() method could throw an exception if there already exists a row with the same key. The question is, is it reasonable to assume that the caller ensures their row is unique before calling InsertRow? If the answer is no, then I'd also provide a bool TryInsertRow() which returns false if the row already exists. In other cases, such as db connectivity errors, TryInsertRow could still throw an exception, assuming that maintaining db connectivity is the caller's responsibility.

Suamere
  • 1,118
Bubblewrap
  • 1,049
28

IMHO returning a "status code" stems from historical times, before exceptions became commonplace in mid- to higher level languages such as C#. Nowadays it is better to throw an exception if some unexpected error prevented your method from succeeding. That way it is sure the error doesn't go unnoticed, and the caller can deal with it as appropriate. So in these cases, it is perfectly fine for a method to return nothing (i.e. void) instead of a boolean status flag or an integer status code. (Of course one should document what exceptions the method may throw and when.)

On the other hand, if it is a function in the strict sense, i.e. it performs some calculation based on input parameters, and it is expected to return the result of that calculation, the return type is obvious.

There is a gray area between the two, when one may decide to return some "extra" information from the method, if it is deemed useful for its callers. Like your example about the number of affected rows in an insert. Or for a method to put an element into an associative collection, it may be useful to return the previous value associated with that key, if there was any. Such uses can be identified by carefully analyzing the (known and anticipated) usage scenarios of an API.

Dan
  • 326
  • 1
  • 8
Péter Török
  • 46,547
14

Peter's answer covers the exceptions well. Other consideration here involve Command-Query Separation

The CQS principle says that a method should either be a command, or a query and not both. Commands should never return a value, only modify the state, and queries should only return and not modify the state. This keeps the semantics very clear and help make code more readable and maintainable.

There are a few cases were violating the CQS principle is a good idea. These are usually around performance or thread safety.

Péter Török
  • 46,547
Andy Lowry
  • 2,422
6

Whatever the path is your going to walk with this. Don't have return values in there for future use (e.g. have functions always return true) this is a terrible waste of effort and doesn't make the code clearer to read. When you have a return value you should always use it, and to be able to use it you should have at least have 2 possible return values.

refro
  • 1,386
1

I used to write lots of void functions. But since I got on the whole method chaining crack, I started returning this rather than void -- might as well let someone take advantage of the return cause you can't do squat with void. And if they don't want to do anything with it, they can just ignore it.

Wyatt Barnett
  • 20,787