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

Refactor currency enum, add currency format method #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KunjPrasad
Copy link

  1. Refactored currency enum to have fields, rather than using a helper method
  2. Added a currency symbol helper method, that delegates the functionality to babel
  3. Added test
  4. Ran black for formatting


class InvalidAmountError(ValueError):
def __init__(self):
super().__init__('Invalid amount for currency')
super().__init__("Invalid amount for currency")
Copy link
Author

Choose a reason for hiding this comment

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

Sigh! Didn't realize.. these changes are after running black on the files

"""Creates a Money instance from sub-units."""
sub_units_per_unit = CurrencyHelper.sub_unit_for_currency(currency)
Copy link
Author

Choose a reason for hiding this comment

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

This is a concrete change. Removed CurrencyHelper. Instead the field is defined for Currency Enum


class Currency(Enum):
Copy link
Author

Choose a reason for hiding this comment

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

Consolidated currency details inside Enum. Previously, the behavior was split between Currency and CurrencyHelper methods

Taken from https://github.com/sebastianbergmann/money
"""
_CURRENCY_DATA = {
"AED": {
Copy link
Author

Choose a reason for hiding this comment

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

The changes are: (1) _CURRENCY_DATA is pulled out from within CurrencyHelper class; (2) The key of dictionary is a string. The format is useful for defining the Currency Enum


@classmethod
Copy link
Author

Choose a reason for hiding this comment

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

Best is to consider it as deletion of entire CurrencyHelper class and making new Enum. The currency properties are now directly associated with the Enum

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.

1 participant