ThreeTen / threeten-extra

Provides additional date-time classes that complement those in JDK 8
http://www.threeten.org/threeten-extra/
BSD 3-Clause "New" or "Revised" License
387 stars 77 forks source link

Non-throwing versions of intersection and union #328

Closed greyhairredbear closed 3 months ago

greyhairredbear commented 5 months ago

When getting the intersection or the union of two Intervals, I either have to check for isConnected beforehand or wrap the call inside a try-catch to avoid a DateTimeException. Both of these solutions

  1. feel a bit cumbersome
  2. might be bad for performance (haven't done any measurements myself, but my educated guess is try-catch might impact performance, the extra isConnected call might negligible).

That's why I'd like to propose to create a version of these functions which return null instead of throwing. I think that this would also make sense in terms of semantics (with null implying "there is no such thing" vs. a DateTimeException implying "you're using this function in the wrong way").

My use case to give a bit more context: I'm using threeten-extra for solving a Vehicle Routing Problem using Optaplanner/Timefold (that's where the concern regarding performance is stemming from) using Kotlin as programming language (that's where the feeling of "cumbersomeness" is stemming from - Kotlin provides quite nice tools to work with nullable values).

I'd also like to understand the reason for implementing intersection and union this way, since for example overlap from joda time returns null instead of throwing when there is no overlap between the intervals.

Willing to open a PR for this, if this feature is wanted.

jodastephen commented 3 months ago

I can say for certain that a method returning null isn't appropriate for ThreeTen-Extra - I consider null return values to be painful.

The question then becomes whether an Optional returning method in these use cases adds enough value, and I struggle to see that it does. There is already the isConnected method, which is little different conceptually to checking the state of the optional.

As such, I'm going to close this as won't fix.