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.

Adding excessive checks in self written code adds clutter but ensures its safe from future modifications.
Reducing checks because 'you know it wont return null' makes code clean, but we live with risk of accidental modification.

There is a nice article here http://blog.ploeh.dk/2013/07/08/defensive-coding/ — Fendy, Aug 12 '13 at 20:09
Moreover, overridding it to return empty / null value violates Liskov Substitution Principle, `Postconditions cannot be weakened in a subtype.` — Fendy, Aug 12 '13 at 20:12
thanks for this helpful, but i was just providing a random example..my question was not example specific — JavaDeveloper, Aug 12 '13 at 20:48
The concept still the same, as stated in the article, more defensive coding makes the code harder to read and null check duplication. Also, the requirement for another check / handle for subtype is clearly violates LSP — Fendy, Aug 12 '13 at 20:59
Protect your code from invalid data coming from “outside”, wherever you decide “outside” is. Data from an external system or the user or a file, or any data from outside of the module/component. Establish “barricades” or “safe zones” or “trust boundaries” – everything outside of the boundary is dangerous, everything inside of the boundary is safe. — JavaDeveloper, Aug 13 '13 at 20:20
http://swreflections.blogspot.com/2012/03/defensive-programming-being-just-enough.html — JavaDeveloper, Aug 13 '13 at 20:21

score 1 · Answer 1 · answered Aug 13 '13 at 14:22

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

write proper javadoc documentation
write proper unit tests
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.

I would like to add that in addition to JavaDoc documentation, it really helps if the code is reviewed before it is finalized. — Maarten Bodewes, Aug 14 '13 at 22:12

score 0 · Answer 2 · answered Aug 12 '13 at 19:38

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.

Excessive checks vs Future extensibility tradeoff

2 Answers2