Thanks for the review Tom!
Really valuable to have floating point experts about - there is a lot to learn there w.r.t. corner cases (so often float behavior does not match intuition - I presume because it is not actually reasonable to have performant representation that is a true behavioral equivalence with real numbers+). As I snag the tip of that iceberg with this general_intervals crate design I got nervous. I really want to avoid assertions / exceptions if at all possible, but that becomes substantially more burdensome (as one example I think I would need to add support for num-traits Checked operations to detect underflow / overflow in Units of Measure?).
My recommendation is to start with the full Interval Enum variant taxonomy and to offer two sets of constructors: strict (your alternatives 1 and 3) and bound-order-independent (your alternatives 2 and 4), both returning an Option. Your alternative 5 is disjoint, so can be added to (or deleted from) the Interval Enum at any time before crate stabilization.
I don’t know why - but I had gone into this thinking I had to settle on just one constructor. I guess because I thought that StructName::new() was such a “reserved keyword” (muscle memory if nothing else) and therefore you needed just one approach. Is it normal to offer multiple constructors and if so what is a nominal naming convention? I’ll go dig through some Crates looking - but would it be something like:
- BoundPair::new_strict_by_bounds(left_bound: T, right_bound: T) -> Option<BoundPair> // Alternative 1
- Option returns None if left_bound >= right_bound
- BoundPair::new_relaxed_by_bounds(one_bound: T, other_bound: T) -> Option<BoundPair> // Alternative 2
- Swaps one_bound, other_bound if necessary when forming left_bound, right_bound
- Option returns None if one_bound == other_bound
- BoundPair::new_strict_left_and_width(left_bound: T, width: T) -> Option<BoundPair> // Alternative 3
- Option returns None if width <= 0
- BoundPair::new_relaxed_left_and_width(one_bound: T, width: T) -> Option<BoundPair> // Alternative 4
- one_bound left / right-ness determined by positive vs negative width
- Option returns None if width == 0
I had hoped that the relaxed alternatives would remove the Option needs, and they almost do - but the Singular Interval scenarios seem to be ruining that? At least if I intend to pass a BoundPair into the Interval Enum variant constructors (as there is no error handling possible in Enum construction?).
Instead of error checking exclusively in the BoundPair construction - I had wondered if there was a method to do so in the Enum constructors? But I wasn’t sure what that would look like (would I need a constructor per variant? And could it then emit a differing type of Enum Variant than requested if for example bounds indicated a Singleton? But are we then hiding from the developer the fact that this was a Singleton when they though it was e.g. LeftHalfOpen?).
Given the mathematically-discontinuous nature of all finite-precision floating point representations, I do not consider strict enforcement of interval width to be generally achievable, so I would ignore that issue during initial development.
I worried about this - good to hear another opinion from a Floating Point expert. I could at least protect against width e.g. underflow / overflow if I manage to get Checked operations working? I’ll de-prioritize dealing with this for now (which was sort of where I had settled in fear of deadlocking and not moving forward due to ratholes).
Whether or not to change the Enum variant on overflow/underflow is a design choice somewhat impacted by the method signature. I recommend using a feature flag to code both ways, then see if one variant has a significant run-time or code-size or usability penalty relative to the other.
I had not considered feature flags and I like that option. I’ll add this to the design doc! Thank you.
Again, thanks for your time Tom!