55

Say you are designing a Square root method sqrt. Do you prefer to validate the parameter passed is not a negative number or do you leave it up to the caller to make sure the param passed is valid. How does your answer vary if the method/API is for 3rd party consumption or if it is only going to be used for the particular application you are working on

I have been of the opinion that a method should validate its parameter, however, Pragmatic Programmer in its Design by Contract section (chapter 4) says it's the caller responsibility to pass good data (pg 111 and 115) and suggests using Assertions in the method to verify the same. I want to know what others feel about this.

2
  • 1
    Assertions that can be removed when code is put into production are a good answer--general validation (except on, perhaps, really critical platforms like a space probe or something) is usually too costly in performance to justify. Why validate parameters to a sqrt() functions I'm going to calls thousands of times a second, when I should just assert out in testing?
    – user18014
    Apr 4, 2011 at 19:41
  • 4
    @WorldEngineer this is not a duplicate of that question. This one is very specific for method arguments. Jul 5, 2017 at 12:15

10 Answers 10

40

In general, I design my APIs as follows:
1. Document the methods well, and encourage the callers to pass good/valid data.
2. Validate the parameters anyway! - throwing exceptions when the preconditions are not met.

I would say that parameter validation is necessary on most public-facing APIs. Parameter validation on non-public methods is not as important - it is often desirable to have validation occur only once, at the public 'entry point' - but if you can live with the potential performance hit, I like to validate parameters everywhere, as it makes code maintenance and refactoring a bit easier.

5
  • 18
    Even for non-public stuff, validation helps when somebody else comes along 5 years later, does not read the comments of docs, and uses something the wrong way. Blowing up (ie exception) is better than some subtle horrible thing that takes hours to find. Apr 4, 2011 at 9:30
  • 10
    Since the extra validation and the documentation and source validation are all separate, this often leads to code rot where all three say something different. There's a surprisingly high risk of problems with this. (I've done enough reverse engineering of legacy applications as part of a rewrite to assert that the code rot is almost inevitable.)
    – S.Lott
    Apr 4, 2011 at 15:15
  • 2
    no performance hit if you only validate in debug builds (for method calls, not for external input...) Apr 4, 2011 at 19:59
  • 4
    I'd prefer to validate in release as well unless profiling showed it was an issue
    – jk.
    Feb 9, 2012 at 11:34
  • @S.Lott Fortunately, that kind of rot is fail-fast. May 29, 2016 at 21:24
37

If you always validate parameters then you are doing extra work that may not be needed.

Think of the situation where the input has already been validated before the call and now you are re-validating the data in the call. OK one extra validation check is OK but now extend that logic to all functions in your application. Every function call validates the data even though it has been validated before (now multiple times).

Data should be validated at ONE point. This is where the data enters the program (or subsystem (this gives some leeway as the definition of subsystem can be flexible)). If there are no programming errors then your code should now work (Note assertions for checking bad code are different from validating parameters).

If you really want to validate parameters then have two versions of the function. One to validate and one that does not validate. Look to std::vector (operator[] does not validate while at() does validate).

So if I had to design a sqrt() function it would not validate its inputs because in the majority of situations the data would be good anyway and the small minority of situations where it is potentially wrong the user can do a quick check (like user input it could be wrong and would need to be validated before use). The only other time it is wrong is for programmer error (and your unit tests should catch these).

10
  • 10
    "Measure twice, cut once" also means extra work that may not be needed. The point is that you usually don't know if it's needed before and when it's needed, it'll save much more time than it costs.
    – nikie
    Apr 4, 2011 at 9:27
  • 3
    I think the comment is fair for time-critical code, or something thats used zillion times a second in a tight loop. Perhaps the moral is: consider the circumstances. Apr 4, 2011 at 9:31
  • 2
    If you write with the intent to re-use, then you should always validate your parameters, as you can never be sure your code will be called safely. I'd argue that the only time you wouldn't validate your parameters is in private methods.
    – TMN
    Apr 4, 2011 at 12:47
  • 5
    To avoid code rot, validate exactly once and never again. Never. If maintenance in one place provides bad data in another place, a total, noisy crash should be the result. Not a second validation and a nice error message. Failure to provide proper data is a design problem, and should be fixed at design time. Not at run time.
    – S.Lott
    Apr 4, 2011 at 15:17
  • 1
    @S.Lott: That's exactly the point: a total noisy crash should be the result. If you're sure that'll happen, everything is fine. But if your sqrt-Algorithm would not crash for negative numbers, but instead return bogus results, then you should better check the inputs and make it crash.
    – nikie
    Apr 6, 2011 at 7:22
22

How much redundancy/robustness should complex software implement? asks a related question.

My answer is that functions which interact with the outside world (public APIs, UI methods, file readers, etc) should validate input and inform the user of errors as politely and clearly as possible. At the very least the error message/return code should indicate where the bad input occurred, and what type of constraint it violated. The more specific the better.

On the other hand, private functions that only deal with internally generated data or data already processed by one of external-facing functions should have assertions on any of the preconditions required for it to operate successfully. If a programmer writes code which violates these, the program should fail and fail hard. This type of bug should never make it past the early testing stages.

The motivation here is to be as nice as possible to the users, while at the same time limiting the decisions on how to handle bad inputs to as high a level as possible. You don't want to be figuring out the next piece of your program-level error handling strategy every time you write a low-level function.

So: validate user input, assert programmer input.

0
11

If you have a type system, use that.

Assertions will help catch evil early.

not-null constraints are a reasonable thing.

sqrt(-1) is not an error in some programming languages. Smalltalk's with complex number support will just return i.

2
  • If the data to be verified can be assembled into an immutable value-object, then the validation can be performed at the constructor, but omitted when the object is copied. (This won't be applicable to the square root function, however. @jv42's answer makes more sense for math functions.)
    – rwong
    Apr 4, 2011 at 12:41
  • Remember to use parameter objects when you have too many parameters... some worthy said that if you methods takes seven paremeters, you've forgotten one. Apr 5, 2011 at 4:46
8

I think it depends on the application of the method, rather than implementation theory.

What I mean is: if you're building a fast math library, even though it might be used by anyone, you don't want to have runtime checks, at least not when built in 'release' mode, because speed is the judging criterion. You might implement checks in a 'debug' mode, using assertions because you want the behavior to be consistent between modes. Of course, you want to document that kind of behavior very well, so the users of your library (even if it's you in three months!) knows what checks they should be doing.

Now, if you're building a network communication library, you want to add as much security as you can, because 1) it will be fed mostly user input, hence danger 2) raw performance will be bound mostly by network I/O, not by CPU operation in most case, so adding some validation won't even be noticed.

2
  • 1
    +1. In general, almost all good answers start with "It depends". Apr 4, 2011 at 15:38
  • @Nemanja Trifunovic You're right :)
    – jv42
    Apr 4, 2011 at 19:31
2

Every function should verify its input, even those internal functions that are not part of any API or public interface.

Programmers are human, and humans are notoriously inept at keeping related constraints in sync as large code bases evolve — eventually, the "check input" part that happens before "call function" will disappear or move elsewhere or become incomplete, and the function will be called with incorrect input. When this happens, your top two objectives will be:

  • Detect the issue as quickly as possible (compile-time being the best option)
  • Don't break anything until the issue is fixed

For many things, you can use language features or the type system to carry compile-time information about what properties have been verified. This is both extremely fast (no run-time penalty) and detects errors at compile-time. Most of my verifications are in this category, for instance.

If your language does not support compile-time verification for what you're doing (which, in modern languages, is quite rare), add a run-time assertion.

Only if the failure of your code could not possibly have adverse consequences beyond an easily detected and innocuous bug, and you expect that code to be called extremely often, and the verification is not a natural part of the function code anyway, you may leave the verifications out. sqrt would probably be here.

2

For C/C++ and other languages that provide functional preprocessor, you could validate input parameters only when build for Debugging/Testing and produce non-validate Release build.

Visual C++ MFC library is the one of good examples.

Below is the sample code from MFC public samples:

void CServerNode::CalcBounding(CDC* pDC, CPoint& ptStart, CSize& sizeMax)
{
    ASSERT(sizeMax.cx >= 0 && sizeMax.cy >= 0);
    ASSERT(ptStart.x >= 0 && ptStart.y >= 0);

    CSize sizeNode;
    CalcNodeSize(pDC, sizeNode);

    ptStart.y += sizeNode.cy + CY_SEPARATOR;
    if (ptStart.y > sizeMax.cy)
        sizeMax.cy = ptStart.y;

    if (ptStart.x + sizeNode.cx > sizeMax.cx)
        sizeMax.cx = ptStart.x + sizeNode.cx;
    ptStart.x += CX_INDENT;
    // add in the kids
    if (!m_bHideChildren)
    {
        POSITION pos = m_listChild.GetHeadPosition();
        while (pos != NULL)
        {
            CServerNode* pNode = (CServerNode*)m_listChild.GetNext(pos);
            pNode->CalcBounding(pDC, ptStart, sizeMax);
        }
    }
    ptStart.x -= CX_INDENT;
}
1

I always put validation close to the source of incoming data, whether that's data from a database, data from a HTTP POST or data from a network socket.

The reasons for this are:

  • reduction of code
  • elimination of unnecessary ops
  • simpler code (usually)

However, there will always be exceptions to most programming rules or best practices. The key in recognizing these exceptions is careful thought and consideration of each situation rather than blind adherence to a set of rules.

1

I think that every function should verify the validity of it's input parameters, and it's better if it can be verified at compile time (with the help of the type system, if you're using a statically typed language).

The idea about using assertions to make sure the input parameters are valid, seems a little bit strange to me - essentially, it means that you have to write the same checks two times - once in the caller function, and the second time in the method itself, in the form of assertions. This also means tha when requirements on the input parameters change, you will have to change the checks everywhere in the calling functions, not just the callee.

So why not just validate the parameters in the method itself and raise an exception (or do whatever is appropriate) when an inconsistency is found?

0

I always use the simple scheme : UI and simple code to call methods (Validate UI parameters) -> Methods (validates not UI parameters) -> Additional Functions (they don't validate anything)

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