Excessive checks vs Future extensibility tradeoff

Question

I can be sure that 'my code' called returnlist() is never going to return null. Thus I do not do any null pointer check when I loop in the enhanced for loop. But, looking from maintenance this does not appear to be a good practice, since someone in future can modify code base inadvertantly or someone would just override this call and overridden function may not be prudent enough to return empty collections.

   public class ExtendedForLoop {

    public List<Integer> returnList() {
        // compute.
        if list == null ? Collections.emptyCollections : list;
    }

    public static void main(String args[]) {
        for (Integer i : returnList()) { 
            System.out.println(i);
        }
}

So making question generic, the question is intended to know best practices of where to draw the tradeoff line. I fully understand there is not defined rule for such cases, just seeking best practices.

1. Adding excessive checks in self written code adds clutter but ensures its safe from future modifications.

2. Reducing checks because 'you know it wont return null' makes code clean, but we live with risk of accidental modification.

Answer 1

That's what javadoc is for:
Just add:

/**
* @return the list, or an empty List. Never returns null!
*/
public List<Integer> returnList()

And further if the code runs in an embedded device installed in 400.000 vehicles you may additonally null check, although you are sure. You want to avoid calling back that devices uinder all circumstances.

If it runs on one server you can rely on correct implementation.

Answer 2

If you care about such aspect of your code's evolution, you should:

1. write proper javadoc documentation
2. write proper unit tests
3. setup a CI environment

Documentation is designed to be consumed by developers, but developers under pressure adopt fast reading techniques that reduce their understanding of the details.

If you code a test asserting all the required properties of your code, you can remove such checks from the clients. However, note that tests have to be mantained. While CI, by default, ensure that all running tests passes, it says nothing about missing tests.

Unfortunately you can't enforce that all required unit tests have been written in a CI process. All you can do, is enforcing a minimum code coverage of 100% to ensure the a minimal number of reviews of the code.
But the code won't be perfect, nevertheless.

The best practice is to do what you are payed for, honestly.
This means that if you are coding a prototype or an application that doesn't need to be reliable, you should not care about such concerns!

On the opposite if you are coding (say) a surgery-robot's controller, you should ensure at least a 100% coverage by test, and fully updated documentation (and still be prepared to cause a few deads).

Reliability is a feature like any other: it should be estimated, payed and measured.