General function question (C++ / Java / C#)

Question

This question is probably language-agnostic, but I'll focus on the specified languages.

While working with some legacy code, I often saw examples of the functions, which (to my mind, obviously) are doing too much work inside them. I'm talking not about 5000 LoC monsters, but about functions, which implement prerequisity checks inside them.

Here is a small example:

void WorriedFunction(...) {
   // Of course, this is a bit exaggerated, but I guess this helps
   // to understand the idea.
   if (argument1 != null) return;
   if (argument2 + argument3 < 0) return;
   if (stateManager.currentlyDrawing()) return;

   // Actual function implementation starts here.

   // do_what_the_function_is_used_for
}

Now, when this kind of function is called, the caller doesn't have to worry about all that prerequisities to be fulfilled and one can simply say:

// Call the function.
WorriedFunction(...);

---

Now - how should one deal with the following problem?

Like, generally speaking - should this function only do what it is asked for and move the "prerequisity checks" to the caller side:

if (argument1 != null && argument2 + argument3 < 0 && ...) {
   // Now all the checks inside can be removed.
   NotWorriedFunction();
}

Or - should it simply throw exceptions per every prerequisity mismatch?

if (argument1 != null) throw NullArgumentException;

---

I'm not sure this problem can be generalized, but still I want to here your thoughts about this - probably there is something I can rethink.

If you have alternative solutions, feel free to tell me about them :)

Thank you.

Answer 1

Every function/method/code block should have a precondition, which are the precise circumstances under which it is designed to work, and a postcondition, the state of the world when the function returns. These help your fellow programmers understand your intentions.

By definition, the code is not expected to work if the precondition is false, and is considered buggy if the postcondition is false.

Whether you write these down in your head, on paper in a design document, in comments, or in actual code checks is sort of a matter of taste.

But a practical issue, long-term, life is easier if you code the precondition and post-condition as explicit checks. If you code such checks, because the code is not expected to work with a false precondition, or is buggy with a false postcondition, the pre- and post- condition checks should cause the program to report an error in a way that makes it easy to discover the point of failure. What code should NOT do is simply "return" having done nothing, as your example shows, as this implies that it has somehow executed correctly. (Code may of course be defined to exit having done nothing, but if that's the case, the pre- and post- conditions should reflect this.)

You can obviously write such checks with an if statement (your example comes dangerously close):

if (!precondition) die("Precondition failure in WorriedFunction"); // die never comes back

But often the presence of a pre- or post-condition is indicated in the code by defining a special function/macro/statement... for the language called an assertion, and such special construct typically causes a program abort and backtrace when the assertion is false.

The way the code should have been written is as follows:

void WorriedFunction(...)  
 {    assert(argument1 != null); // fail/abort if false [I think your example had the test backwards]
      assert(argument2 + argument3 >= 0);
      assert(!stateManager.currentlyDrawing());
      /* body of function goes here */ 
 }  

Sophisticated functions may be willing to tell their callers that some condition has failed. That's the real purpose of exceptions. If exceptions are present, technically the postcondition should say something to the effect of "the function may exit with an exception under condition xyz".

Answer 2

That's an interesting question. Check the concept of "Design By Contract", you may find it helpful.