Protect Rational with 0 numerator from dividing by 0 in respelling(numerator:)

[jsbean]

The Rational respelling(numerator:) and respelling(denominator:) methods use the / operator, which in turn uses the reciprocal property.

This then turns a Rational with zero numerator to one with a zero denominator. Which crashes.

Check if numerator == 0, then exit early with an easy operation on the denominator.

[mossheim]

in the case of respelling(numerator:), should it return nil if the requested numerator is anything other than 0?

[jsbean]

If I understand correctly, it should be something like:

func respelling(denominator: Int) -> Self? {

    guard numerator != 0 else {
        return nil
    }

    // same implementation as before
}

[mossheim]

but the description is

    /// - returns: Representation of a `Rational` value with a given `numerator`, if possible.
    /// Otherwise, `nil`.

so since the rational can't be represented with the new numerator, it should return nil (unless the requested numerator is also 0). right?

[mossheim]

still confused.. can you update those snippets to show when you're using newNumerator vs numerator? The function signature is public func respelling(numerator newNumerator: Int) -> Self?

[jsbean]

Ahh, I see. I was really sloppy and incorrect previously.

It is only applicable for respelling(denominator:), where the current numerator is 0.

    /// - returns: Representation of a `Rational` value with a given `denominator`, if
    /// possible. Otherwise, `nil`.
    public func respelling(denominator newDenominator: Int) -> Self? {

        guard numerator != 0 else {
            return Self(0, newDenominator)
        }

        guard newDenominator != denominator else {
            return self
        }

        let quotient = Float(newDenominator) / Float(denominator)
        let newNumerator = Float(numerator) * quotient

        guard newNumerator.truncatingRemainder(dividingBy: 1) == 0 else {
            return nil
        }

        return Self(Int(newNumerator), newDenominator)
    }

[jsbean]

Another option is to make these not return an Optional<Self>, but just make these assert. Then, these treat programmer errors in the same way as the init.

So far, every time these are used, they are force unwrapped. That seems like a sign to make them assert, not return an optional.

[jsbean]

Also, we might as well make these Float values Double values.