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

Improve names for functions and state variables #3

Open
JacobVanGeffen opened this issue Aug 15, 2024 · 3 comments
Open

Improve names for functions and state variables #3

JacobVanGeffen opened this issue Aug 15, 2024 · 3 comments

Comments

@JacobVanGeffen
Copy link
Collaborator

JacobVanGeffen commented Aug 15, 2024

Inappropriately named functions/variables should be changed to follow these rules:

  • Function names should reflect whether or not they update state (e.g. functions prefixed with check should not update state).
  • Nouns should be used to describe getter functions.
  • Verbs should be used to describe state-affecting functions.
  • State variable names should describe the class of data that the variable pertains to (e.g. prefix or postfix with ip when a variable only describes in-position data).

For example, check_card_config should be renamed to reflect the fact that it updates state through init_hands.

Additionally, there is currently a convention of state variables having either no postfix to mean "applies to out-of-position" and postfix _ip to mean "applies to in-position". Variables representing out-of-position traits should be postfixed with _oop to make this distinction clear.

@JacobVanGeffen JacobVanGeffen changed the title Improve names for functions Improve names for functions and state variables Aug 15, 2024
@bkushigian bkushigian assigned bkushigian and unassigned bkushigian Aug 16, 2024
@bkushigian
Copy link
Owner

bkushigian commented Aug 16, 2024

Agreed!

I think that some of the _ip postfix vs no postfix isn't quite so straightforward. For instance PostFlopNode::num_elements is always the number of 'elements' the current player has, while PostFlopNode::num_elements_ip is the number of elements IP has, but only on the first node of a street.

It would be good to:

  1. figure out exactly why this is needed
  2. rename accordingly
  3. document :)

@bkushigian
Copy link
Owner

We should put this into the docs branch as that is where I'm keeping track of all new documentation and clarity renaming changes.

@bkushigian
Copy link
Owner

Now part of PR #4

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

2 participants