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

Should the deck methods return <DeckOfCards> object instead of returning <List<Card>> ? #2

Open
sammysignal opened this issue Apr 25, 2021 · 2 comments

Comments

@sammysignal
Copy link

I noticed that the documentation indicates that methods add_deck, order_deck, shuffle_deck, reset_deck are returning deck objects, but that's not actually the case - they are instead returning a list of cards which is a bit confusing. It may be fine to return a list of cards, as long as the docs are updated, however, you could also instead simply return self. If we return self we could also write a __getitem__ function so that you can index directly into the cards object (e.g., card_obj[0]) - that way it would feel more like a list.

happy to make a pr for either option, curious which way you prefer.

@SimonTheChain
Copy link
Owner

Thank you for this finding! Your idea to return self sounds interesting; I'm not used to overriding __getitem__ so I'd say go for it if you think that's the better way.

I'm just concerned about one thing: if there are people who already use this module, would they need to change their code after the update?

Honestly it's very exciting to have other people use this module so thank you for looking into this!

@sammysignal
Copy link
Author

Cool I just made a PR for it, yes I do think its better to return self but its not a big deal either way. PR also includes making the deck iterable and indexable.

And I don't think clients would have to change their code - you could make a new release and they would only have to change their code after upgrading the module using pip3 install --upgrade or something like that.

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