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

Palindrome products: sync expected test results and input data with problem-specifications. #1814

Merged
merged 8 commits into from
Jun 17, 2019

Conversation

BethanyG
Copy link
Member

@BethanyG BethanyG commented Jun 1, 2019

Part of #1762

  • Changed function names to largest / smallest in tests, example.py, and exercise stub to conform with canonical-data.json.
  • Changed main function function argument names to min, / max in tests, example.py, and exercise stub to conform with canonical-data.json.
  • Changed expected data type in tests to list of lists to conform with canoncial-data.json.
  • Changed some function and variable names in example.py to better clarify code and remove single letter variable names.
  • Removed from __future__ import division from example.py

@BethanyG BethanyG requested a review from a team as a code owner June 1, 2019 21:17
@@ -1,53 +1,51 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

def largest_palindrome(max_factor, min_factor):
return get_extreme_palindrome_with_factors(max_factor, min_factor,
"largest")
def largest(min, max):
Copy link
Contributor

Choose a reason for hiding this comment

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

min and max are reserved.

Suggested change
def largest(min, max):
def largest(min_factor, max_factor):

@@ -1,10 +1,10 @@
"""
Notes regarding the implementation of smallest_palindrome and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Notes regarding the implementation of smallest_palindrome and
Notes regarding the implementation of smallest and

@BethanyG
Copy link
Member Author

BethanyG commented Jun 13, 2019

Since I needed to alter the tests as well, decided to push additional changes.

  • Changed min/max back to min_factor / max_factor in tests, test notes, example code, and exercise stub to avoid using min/max reserved words for argument names.
  • Changed reference to smallest_pallendrome to smallest in test notes.

@cmccandless cmccandless merged commit 88cd48b into exercism:master Jun 17, 2019
@cmccandless
Copy link
Contributor

Merged; thanks for working on this!

@BethanyG BethanyG deleted the palindrome-products-changes branch January 29, 2021 17:53
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

Successfully merging this pull request may close these issues.

2 participants