vavr icon indicating copy to clipboard operation
vavr copied to clipboard

Remove explicit null checks

Open patrickguenther opened this issue 2 years ago • 3 comments

https://github.com/vavr-io/vavr/issues/2711

patrickguenther avatar May 06 '23 15:05 patrickguenther

What is the reasoning behind this change? At first glance it seems to cloak a lot of programming mistakes by shifting away from fail-early-at-root-cause to fail-at-random-point-in-time.

szarnekow avatar May 06 '23 16:05 szarnekow

@szarnekow see discussion in #2711 But also: the whole shift we see in the last couple of years for programming languages becoming more functional is because we, the software developer community, have figured out that the best solution to avoid NPEs is to completely eradicate the use of the nullpointer itself. If that goal is reached, however, all code for checking nulls just becomes obsolete boilerplate.

The other argument, you brought up: "Throwing close to the source" is maybe only valid when the potential throwing of the NPE and the injection point of the null value are in completely different execution stacks. For that, I would support placing some null checks in strategic places like Futures, where an uncaught NPE might even lead it to never terminate. Otherwise IDEs usually highlight your own classes in the stack trace or when looking at the stacktrace in a terminal or textfile you anyway quickly scan the trace for your own namespace.

patrickguenther avatar May 06 '23 20:05 patrickguenther

What is the reasoning behind this change? At first glance it seems to cloak a lot of programming mistakes by shifting away from fail-early-at-root-cause to fail-at-random-point-in-time.

See the discussion of this in #2711. Evidently the reasoning is that it is for performance reasons that the null checks are being removed. Ideally null checks should not be needed in a functional library as nulls have no business being in functional code—but unfortunately in Java that minefield always lurks under the surface. Personally I think it’s ok to remove the null checks because 1) there shouldn’t be any nulls when your code properly protects against them coming from 3rd party libraries, and 2) a resulting NPE almost always provides enough locality info to quickly determine where the null came from.

For me the compelling reason is to keep the code cleaner by removing the null-check noise; I am skeptical about performance being a compelling reason to remove them.

ezoerner avatar May 06 '23 20:05 ezoerner