Skip to content

Create problem_54 in project Euler - #2451

Merged
cclauss merged 7 commits into
TheAlgorithms:masterfrom
dhruvmanila:master
Sep 20, 2020
Merged

Create problem_54 in project Euler#2451
cclauss merged 7 commits into
TheAlgorithms:masterfrom
dhruvmanila:master

Conversation

@dhruvmanila

@dhruvmaniladhruvmanila commented Sep 19, 2020

Copy link
Copy Markdown
Member

Add solution and test files (in pytest) for project Euler problem 54

There are a couple of extra features of the PokerHand class:

  • A public method PokerHand.hand_name() which returns the name of hand as stated in the Wikipedia link
  • Added rich comparison operators such as <, >, <=, >=, ==, != and this in turn gives us the ability to use the builtin sort method and sorted function on a list of PokerHand objects.

The test file test_poker_hand.py should be picked up by Travis and it checks every function of sol1.py through pytest.

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

@dhruvmanila

dhruvmanila commented Sep 19, 2020

Copy link
Copy Markdown
MemberAuthor

Hi, I'll fix the codespell errors.

What does this codespell comment trying to say?

'project_euler/problem_54/test_poker_hand.py': [(209, 'ans', 'and'), (217, 'ans', 'and'), (218, 'ans', 'and')]}

Should I change the variable named ans to something more verbose like answer? This variable is in test_poker_hand.py file.

cclauss
cclauss reviewed Sep 19, 2020
View reviewed changes
Comment threadproject_euler/problem_54/sol1.py Outdated
cclauss
cclauss reviewed Sep 19, 2020
View reviewed changes
Comment threadproject_euler/problem_54/sol1.py
cclauss
cclauss reviewed Sep 19, 2020
View reviewed changes
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
cclauss
cclauss reviewed Sep 19, 2020
View reviewed changes
Comment threadproject_euler/problem_54/sol1.py Outdated
cclauss
cclauss reviewed Sep 19, 2020
View reviewed changes
Comment threadproject_euler/problem_54/sol1.py Outdated
@dhruvmanila
Commit suggestions:
fa9c267
- Use list comprehension instead of map
- Sort imports using isort
@cclauss

cclauss commented Sep 19, 2020

Copy link
Copy Markdown
Member

Should I change the variable named ans to something more verbose like answer?

Yes.

cclauss
cclauss reviewed Sep 20, 2020
View reviewed changes
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
cclauss
cclauss reviewed Sep 20, 2020
View reviewed changes
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
cclauss
cclauss reviewed Sep 20, 2020
View reviewed changes
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
cclauss
cclauss reviewed Sep 20, 2020
View reviewed changes
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
cclauss
cclauss approved these changes Sep 20, 2020
View reviewed changes

@cclausscclauss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so cool!

Comment threadproject_euler/problem_54/sol1.py Outdated
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
@dhruvmanila
Changes made as suggested (simplified a lot):
d9a84f2
- List and set comprehension instead of itemgetter
- Using enumerate as it's easy to read
- Divided into list of card values and set of card suit as set will
remove all the duplicate values. So, no need for double indexing.
- Add test for testing multiple calls to five_high_straight function
cclauss
cclauss reviewed Sep 20, 2020
View reviewed changes
Comment threadproject_euler/problem_54/test_poker_hand.py Outdated
@dhruvmanila
Add suggestions and simplified:
781fa33
- Split generate_random_hands function into two:
- First will generate a random hand
- Second, which will be called, will return a generator object
@cclauss
cclauss merged commit ea0759d into TheAlgorithms:masterSep 20, 2020
@cclauss

cclauss commented Sep 20, 2020

Copy link
Copy Markdown
Member

An awesome contribution! Thanks for doing this.

stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
@dhruvmanilagithub-actions@stokhos
Create problem_54 in project Euler (TheAlgorithms#2451)
5c82605
* Add solution and test files for project euler 54
* Update sol1.py
* updating DIRECTORY.md
* Fix: use proper path to open files
* Commit suggestions:
- Use list comprehension instead of map
- Sort imports using isort
* Changes made as suggested (simplified a lot):
- List and set comprehension instead of itemgetter
- Using enumerate as it's easy to read
- Divided into list of card values and set of card suit as set will
remove all the duplicate values. So, no need for double indexing.
- Add test for testing multiple calls to five_high_straight function
* Add suggestions and simplified:
- Split generate_random_hands function into two:
- First will generate a random hand
- Second, which will be called, will return a generator object
Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cclausscclausscclauss approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dhruvmanila@cclauss