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

Round object has to be defined with a name #15

Closed
6 tasks
jatkinson1000 opened this issue Mar 13, 2023 · 2 comments
Closed
6 tasks

Round object has to be defined with a name #15

jatkinson1000 opened this issue Mar 13, 2023 · 2 comments
Assignees

Comments

@jatkinson1000
Copy link
Owner

jatkinson1000 commented Mar 13, 2023

At present a Round object has to defined with a name and a list of Passes.
However, for abstract use we may just want to define passes and no name.
Consider a refactor to make name an optional argument.

Will have knock on effects where Round is used as calls will need to change.
Breaking change?

Requires:

  • Review how much of a breaking change this will be
  • Update Rounds.py Round class
  • Update Rounds tests
  • Update knock on effects in rest of code
  • Update any archerycalculator code affected
  • Notify of changes in docs?
@jatkinson1000
Copy link
Owner Author

After working to implement this I think this is actually not an issue and should be left as it is.

The resulting code for pre-defined rounds is less intuitive with a mass of passes required before setting a name:

york = Round(
    [
        Pass.at_target(72, "5_zone", 122, (100, "yard"), False),
        Pass.at_target(48, "5_zone", 122, (80, "yard"), False),
        Pass.at_target(24, "5_zone", 122, (60, "yard"), False),
    ],
    "York",
)
hereford = Round(
    [
        Pass.at_target(72, "5_zone", 122, (80, "yard"), False),
        Pass.at_target(48, "5_zone", 122, (60, "yard"), False),
        Pass.at_target(24, "5_zone", 122, (50, "yard"), False),
    ],
    "Hereford",
)

I'm also not convinced that there is much case to define without a name - if using we will always be assigning the Round to a variable, so why not assign a "name" at least the same as the variable name.*
And in, e.g. an app, users creating custom rounds to be added to a database would want to give them a name so they can re-use them in future.

* Only case I can see is if defining the round as part of the the function call to elsewhere, but I would argue this is bad usage of the code.

I will let this sit for a little longer but will close on consideration after the current WIP MRs are complete.

@jatkinson1000 jatkinson1000 self-assigned this Feb 25, 2024
@jatkinson1000
Copy link
Owner Author

Closing as discussed above.

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