Page tree
Skip to end of metadata
Go to start of metadata

The current Coding guidelines states that

Public methods should always check that their arguments follow the JavaDoc restriction with respect to being null, empty, non-negative etc. The ArgumentNotValid class has a number of useful methods for this.

What does this mean exactly?. Should object argument always be Checked for null, should the Javadoc always state whether a Object null value is allowed?, should Javadoc always state whether Collections and Array empty values are allowed? I think this statement should either be elaborated or changed to a non-binding describing of the relevant considerations, preferably by providing a link or two to web pages describing our intent. 

  • No labels

3 Comments

  1. I just discussed this with Colin who argued that the quicker we could catch Invalid arguments and generate a specific exception describing precisely what is being violated the better. In think this is a nice idea in theory, but in real life this would lead to a a lot of defensive coding attempting to add better failure handling capabilities bloating the code base with functionality not related to any of the primary functionality.

    Java already provides powerful debugging features like error stack traces pointing you to the precise location of errors and multiple runtime exception types which provide a very good mechanism for failing fast with adequate information in error situations.

    Example (taken from the IndexAggregator class, where Jonas commented on the lake of validation during the review)

    public void sortAndMergeFiles(File[] files, File outputFile) {
        processFiles(files, outputFile, null);
    }
    
    public void mergeFiles(File[] files, File outputFile) {
        List<String> args = new LinkedList<String>();
        args.add("-m");
        processFiles(files, outputFile, args);
    }
    
    private void processFiles(File[] files, File outputFile, List<String> additionalArgs) {
       if (files.length == 0) {
                return;
       }
    ....

    This is the current code. Here you could check the arguments for null which would add 4 new lines of code significant increasing the complicity of this small piece of code, where the only benefit would be throwing a ArgumentNotNull exception instead of a NullPointerException of a a line or two later.

  2. If null-values are acceptable, it should be noted in the javadoc, because generally they shouldn't be IMHO.

    I prefer not to let NPE be thrown in our code, because I regard those as a error in using our methods, and should thrown at once.

    Argument validation can also be seen as a way of stating the precondition of each method, and what is expected of the arguments: E.g. should the File object already exist, and are negative arguments accepted, and so on.

    1. I agree that a set of general argument restrictions would be very useful An example would be that not mentioning null values in the javadoc implies null is an illegal argument. Another could be that empty collection and arrays are valid arguments and return values as default. 

      Is such a set of default argument restrictions specified anywhere in the current documentation?. A natural place to locate such information would be the top of the javadoc, as this would be part of the system level API specification.  

      Another question is when to make explicite null checks. I agree null values should be prevented from burrowing to deep into the runtime data, but often the NullPointerPointer would be thrown quickly anyway. Take for example the code snippet above, where should files argument be checked? If no null tjek is performed, a NullPointer would be throw in the first line of the processFiles method pointing to a specific variable as the sinner.

      Should a null check be performed to prevent the sortAndMergeFiles and mergeFiles from sending null files on to the processFiles method?