7

If I'm building private utility functions, should they be held to the same rigorous standards in terms of handling invalid data as public functions?

Example:If I'm writing code to calculate the length of a linked list, and the list being passed is one created by my own code, should it be checked for loops if my code does not create linked lists with loops in the 1st place?

5 Answers 5

9

Naturally, private methods have just as much to gain as public methods do, from checking to make sure that every single bit of their input is correct. Generally, no programmer would ever be asking themselves the question of whether more checks are better than fewer checks, (duh!) if it was not for performance concerns.

Now, when it comes to checking for invalid stuff there are two types of checks that you can perform:

  • Production-time checks, (in short, runtime checks) which happen both on development and on production environments. (Out there on the field.)

  • Development-time-only assertions, (in short, assertions) which happen only in development environments and are skipped on production environments.

Assertions have the mind-bogglingly useful characteristic of not incurring any performance penalty on production environments, so they can be used very liberally. Go ahead and assert everything you can, it does not affect performance. On the contrary, asserting everything is good for robustness, it is good for debugging, it is good for documentation, it is good for keeping the state complexity of your program down; in short, it is the best thing since sliced bread. So:

The question you should always be asking

is not "should I assert this?"

but "is there anything I forgot to assert?"

So, in a well designed system, both private methods and public methods check everything there is to check, the only difference being the following:

  • All (well, almost all) of the checks performed by private methods are assertions.

  • Many of the checks performed by public methods are runtime checks, (not assertions,) if the public interface of the object demands so. Generally, these checks are for things that may conceivably happen under normal usage and they are not necessarily bugs.

For example, if you are coding a File class, then a failure to open the file is generally not a bug, so it should be checked against and generate an exception even on production. On the other hand, if someone passes your public file-open method a null filename, that is a bug in the caller's code, so the check for a null filename can be an assertion.

And generally, every single thing which gets checked with a runtime check in a public method can also be checked again in a private method, but this time with an assertion, since all input ought to have already been validated by the public methods, so anything bad reaching a private method means you have a bug in your code.

So, in general:

  • Nothing should ever go unchecked, either in public or in private methods.

  • Assertions are free, so use them liberally.

DISCLAIMER: Since this is an interview question, please bear in mind that the above may, and may not, be what the interviewer would like to hear from you. In such cases it is best to prefix your answer with a disclaimer like "Well, I would of course follow whatever discipline is generally used in the house, but if it was completely up to me, then I would ..."

3
  • 1
    Good answer, and you are certainly right when you say "in general", my debug build should at least toddle, if it cannot run. Some pre-conditions are impossible to test (is that a valid pointer, to an object of the right kind?) or prohibitively expensive (Checking for duplicates in a data-structure when searching for an entry). Performance requirements are even more stringent on private than public functions, and there's presumably slightly more trust. Commented Jun 11, 2015 at 17:12
  • 1
    @Deduplicator Yes, I tried to be careful enough to avoid proclaiming any maxims. But I am the guy who will assert that an array is sorted (big-oh of N) before he performs a binary search on it (big-oh of log-N.) Because the assert costs nothing in the runtime, but the bug of not finding an item which is in the array, will cost hours of debugging time.
    – Mike Nakis
    Commented Jun 11, 2015 at 17:36
  • Well, as long as you use an extra debug-level for the truly atrociously expensive checks (adjusted for potential cumulative effects), that's all to the good. Commented Jun 11, 2015 at 17:39
4

YES. Private methods should be held to the same standards as public ones.

Recall that there is a Bell Curve.

Recall that about 50% of programmers are below average. By definition.

Assume that you will NOT be the only person who ever works on your code.

Assume that at least one of the people who works on your code will be someone who should not be allowed access to even Dartmouth BASIC on a standalone minicomputer.

Now, are you STILL sure there will never be any loops in your lists?

I didn't think so.

5
  • 50% below average is a rather interesting statement :)
    – user87166
    Commented Feb 15, 2015 at 7:54
  • @user87166, the basic principle is that most things in life follow a normal distribution, a bell-shaped curve. The curve is symmetric about the mean, which in everyday life we call the "average". Half of the curve is left of the average, below average, half is above average. (TECHNICALLY, half are below the median. For everyday purposes on programming skills, the median and the mean are probably close enough to be considered equal.) Commented Feb 15, 2015 at 16:45
  • +1 for mentioning that you won't be the only person working on the code. It's also useful to mention that these checks and balances are also aimed at your Future Self, who six months later is little better at interpreting your code than a complete noobie. You might know right now to not use circular references. A year from now? Not so much. Commented Jun 11, 2015 at 13:54
  • 1
    Validation of arguments will not prevent a bad programmer from introducing bugs. If they can change the code to introduce the bug, what prevents them from changing the verification code in the same class correspondingly? In any case unittests are a much better way to prevent against introducing bugs.
    – JacquesB
    Commented Jun 11, 2015 at 15:01
  • Unit tests will also not "prevent a bad programmer from introducing bugs". To extend JacquesB's argument, if they can change the code (and "verification code") to introduce the bug, what prevents them from changing the unit test correspondingly? The reality is that "verification code" and unit tests are both useful, and should be used, but neither will prevent against "bad programmers".
    – Eric King
    Commented Jun 11, 2015 at 17:48
3

I will disagree with the other answers and say no. Verifying input is much more important in public than in private methods.

Especially if you write a component for reuse, you cannot make any assumptions about what values are passed to the public methods, except what is guaranteed by the type system.

For private methods it is a different matter, since you can easily see everywhere the method is called and with what arguments, at least if your class have a sensible size. If you never assign null to an object reference for example, you don't need to check for null in a private method. In a public method you cannot make such assumptions. Your example where you know you never create cycles in a linked list is the same.

Of course there is always the risk that somebody modifies another method in the same class to break the assumptions, but then again if you have mad monkeys changing the code at random, you also risk they delete the verification checks anyway. Unit tests are the right approach to verify the integrity of a class and protect against changes that introduces bugs.

In theory it would be nice if all methods verified all input. but introducing verification also have cost. For example a simple extract-method refactoring will require you to copy paste input verification from the host method, introducing code weight for no clear benefit.

1
  • Actually, even in public methods checking all pre-conditions can be impossible (is that a valid pointer, to an object of the right kind?) or prohibitively expensive (Checking for duplicates in a data-structure when searching for an entry). Performane requirements are even more stringent on private functions, and there's presumably slightly more trust. Commented Jun 11, 2015 at 17:06
2

Yes.

A private, internal function is called by some other function (or is dead code).

While a private function is not part of a public interface for a class, namespace, or module, it still can produce useful work for functions that are part of the public interface. While not exposed directly, it does contribute to the public interface transitively.

Private functions need to be tested, documented, and coded to the same standards as public functions. The only standard I would be lax on is documentation: there should be a quick note about what the function does, pre and post conditions, but there is no need to go into as much detail as with public documentation because a private/internal function does not need to be understood by developers outside your team.

1

Write the minimum amount of code to do the job properly. In your example I'd say don't bother, if there is no way (without code modifications) that it make loops then don't bother accounting for it. You'll end up writing less code which you can expand on later, rather than too much code which could potentially get confusing later on.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.