Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors for bet_size.rs #16

Closed
3 tasks done
bkushigian opened this issue Oct 13, 2024 · 1 comment
Closed
3 tasks done

Refactors for bet_size.rs #16

bkushigian opened this issue Oct 13, 2024 · 1 comment

Comments

@bkushigian
Copy link
Owner

bkushigian commented Oct 13, 2024

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.

Summary of proposed changes (details below)

  • Move is_raise related checks from BetSize to BetSizeOptions
  • Make BetSizeOptions fields non-public
  • Renate BetSizeOptions fields (pluralize)

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

These are plural, so these should be pluralized.

bkushigian added a commit that referenced this issue Oct 16, 2024
@bkushigian
Copy link
Owner Author

Closed by #17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant