-
-
Notifications
You must be signed in to change notification settings - Fork 473
Closed
Labels
E-questionParticipation: opinions wantedParticipation: opinions wanted
Milestone
Description
Many distributions currently panic when given invalid parameters:
impl Exp {
/// Construct a new `Exp` with the given shape parameter
/// `lambda`. Panics if `lambda <= 0`.
#[inline]
pub fn new(lambda: f64) -> Exp {
assert!(lambda > 0.0, "Exp::new called with `lambda` <= 0");
Exp { lambda_inverse: 1.0 / lambda }
}
}
In contrast, quite a few parts of the library return a Result
from the ctor. Notably:
impl<X: SampleUniform + PartialOrd> WeightedIndex<X> {
/// Creates a new a `WeightedIndex` [`Distribution`] ...
pub fn new<I>(weights: I) -> Result<WeightedIndex<X>, WeightedError> where ... {}
}
/// Error type returned from `WeightedIndex::new`.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum WeightedError {
/// The provided iterator contained no items.
NoItem,
/// A weight lower than zero was used.
NegativeWeight,
/// All items in the provided iterator had a weight of zero.
AllWeightsZero,
}
@vks suggested adding try_new
ctors to avoid breakage, but due to #290 proposing to move these to rand_stat
there will be some breakage anyway. So which is better?
fn new(...) -> Self
andfn try_new(...) -> Result<Self, ...>
ctorsfn new(...) -> Self
only (status quo) — not ideal for parameters which must be checked at run-timefn new(...) -> Result<Self, ...>
only — requires error handling or.unwrap()
I'm leaning towards the latter despite the breakage. Lots of redundant API functions just for slightly better ergonomics isn't good style and I don't think the breakage is a big deal.
Metadata
Metadata
Assignees
Labels
E-questionParticipation: opinions wantedParticipation: opinions wanted