Unify length overflow checks for builders

[mkrasnitski]

No description provided.

[kangalio]

I think there should be a function to do the checking fn check_overflow(len: usize, max: usize, err: fn(usize) -> ModelError) -> Result<()>, called e.g. like check_overflow(length, crate::constants::EMBED_MAX_LENGTH, ModelError::EmbedTooLarge)?;. It

• reduces code
• allows for comments on the saturating_sub technique (it took me a few seconds to get how it works; would have been easier with comments)
• allows to replace overflow check back to the old code because it optimizes better if we'd care about that

Some overflow errors (EmbedAmount) don't contain the overflow number, but we're on next anyways so we can fix the inconsistency, and then use our new function for those errors as well

[mkrasnitski]

A better API might be fn check_overflow(len: usize, max: usize) -> StdResult<(), usize>, which can then be combined with .map_err. This way we can avoid requiring that every overflow error type wrap the overflow amount.

[kangalio]

.map_err(|overflow| Error::Model(ModelError::EmbedTooLarge(overflow))) is longer than , ModelError::EmbedTooLarge though, plus why not add the overflow amount to errors?

In fact this is what delights me about programming; code simplification naturally leading you to find and fix API inconsistencies and paper cuts.

[mkrasnitski]

My thoughts are that embeds and stickers are limited to such low numbers, that indicating to the user how much they've gone over the limit isn't very useful. Nobody is going to be trying to send 15 embeds in the same message. It makes much more sense when limiting text length, because there can be a lot more of it, and the limit is high, therefore it's useful to tell the user exactly by how much they've gone over.

EDIT: Also, TIL that Enum::Variant(ty1, ty2, ...) implements Fn(ty1, ty2, ...) -> Enum.

[kangalio]

I feel different but it's not objective and not worth bikeshedding over

[mkrasnitski]

Yes, and if those changes were to be made, it should be done in a separate PR. The main goal of this PR is ensuring overflow checks are always correctly performed.