I'm in the midst of writing up some refactors. To make it easier to understand why I'm doing this (especially since there will be some breaking changes introduced), I'm documenting both what I'm changing and why I'm changing it.
Move is_raise related checks from BetSize creation to BetSizeOptions creation
bet_size_from_str(&str, boolean) depends on an is_raise: boolean predicate to return an Error under two conditions:
the resulting BetSize was BetSize::PrevBetRelative
the resulting BetSize was BetSize::Additive(_, cap) with cap != 0
However, creating the bet size shouldn't be where this triggers an Err. For instance, BetSize::PrevBetRelative is a valid bet size but doesn't belong in BetSizeOptions::bet. This is causing some issues. For instance, during deserialization, I don't want to have to encode into the deserialization routine whether we are parsing a bet size or a raise size. Additionally, this is just suboptimal design, and should be refactored in its own right.
Instead, I'm refactoring so that BetSizeOptions is created from BetSizeOptions::try_from_sizes(bets: Vec<BetSize>, raises: Vec<BetSize>). This checks if there are invalid sizes in bets and, if so, returns an Err. Thus, BetSizeOptions should be created through officially supported construction functions such as try_from_sizes, and these should always check for invalid BetSizes in the bet field.
Make BetSizeOptions fields non-public
Next, bet_size::BetSizeOptions has public fields, meaning that it can be constructed:
BetSizeOptions { bet: SOME_VEC, raise: SOME_VEC }
Since there are invalid bet vectors, this means a user can bypass the 'official' constructors for BetSizeOptions (namely, BetSizeOptions::try_from_sizes(...)) which will silently create invalid instances.
Rename BetSizeOptions fields bet and raise to bets and raises
Refactors for
bet_size.rs
I'm in the midst of writing up some refactors. To make it easier to understand why I'm doing this (especially since there will be some breaking changes introduced), I'm documenting both what I'm changing and why I'm changing it.
Move
is_raise
related checks fromBetSize
creation toBetSizeOptions
creationbet_size_from_str(&str, boolean)
depends on anis_raise: boolean
predicate to return anError
under two conditions:BetSize
wasBetSize::PrevBetRelative
BetSize
wasBetSize::Additive(_, cap)
withcap != 0
However, creating the bet size shouldn't be where this triggers an
Err
. For instance,BetSize::PrevBetRelative
is a valid bet size but doesn't belong inBetSizeOptions::bet
. This is causing some issues. For instance, during deserialization, I don't want to have to encode into the deserialization routine whether we are parsing a bet size or a raise size. Additionally, this is just suboptimal design, and should be refactored in its own right.Instead, I'm refactoring so that
BetSizeOptions
is created fromBetSizeOptions::try_from_sizes(bets: Vec<BetSize>, raises: Vec<BetSize>)
. This checks if there are invalid sizes inbets
and, if so, returns anErr
. Thus,BetSizeOptions
should be created through officially supported construction functions such astry_from_sizes
, and these should always check for invalidBetSize
s in thebet
field.Make
BetSizeOptions
fields non-publicNext,
bet_size::BetSizeOptions
has public fields, meaning that it can be constructed:Since there are invalid
bet
vectors, this means a user can bypass the 'official' constructors forBetSizeOptions
(namely,BetSizeOptions::try_from_sizes(...)
) which will silently create invalid instances.Rename
BetSizeOptions
fieldsbet
andraise
tobets
andraises
These are plural, so these should be pluralized.