Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider adding isIntervalEmpty() default method to Interval #315

Open
tpietzsch opened this issue May 24, 2022 · 1 comment
Open

Consider adding isIntervalEmpty() default method to Interval #315

tpietzsch opened this issue May 24, 2022 · 1 comment

Comments

@tpietzsch
Copy link
Member

Currently, to indicate empty Intervals we use the condition that max < min (for some dimension) to indicate that an interval is empty.

The motivation for this is that it "works naturally" for intersection of intervals. The intersection of two intervals has the min at the maximum of the mins of the intersected intervals, and the max at the minimum of the maxs of the intersected intervals. When two non-overlapping intervals are intersected, or one ore both of the intersected intervals are empty, the result is automatically an empty interval by the above condition.

However, for union of intervals we have to be careful. Naturally, the union of two intervals has the min at the minimum of the mins of the unioned intervals, and the max at the maximum of the maxs of the unioned intervals.

However, if one or both of the intervals is empty (by the above condition), this may lead to incorrect results (i.e., the union of two empty intervals can become non-empty). Therefore we need additional isEmpty() checks. These may become costly, in particular if the unioned intervals need to do additional work to compute their respective min and max. See imglib/imglib2-roi#60.

Therefore, we should consider, adding a default boolean isIntervalEmpty() method to the Interval interface. The default implementation could check the max < min condition, but subclasses could implement the check for example by pre-computing a boolean isEmpty flag. The Intervals methods (e.g. Intervals.union()) would then use this method to implement the checks.

The default method should be called isIntervalEmpty instead of just isEmpty because it will be inherited for example by RandomAccessibleInterval, where the meaning of isEmpty would not be immediately clear.

See #301, #300, imglib/imglib2-roi#60

@tischi
Copy link

tischi commented May 25, 2022

I fully support adding default boolean isIntervalEmpty(), in fact when trying to fix imglib/imglib2-roi#60 I played around adding such a method and I agree that it will probably help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants