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 exception handling in color lookup to use conditional check #181

Closed
maxsos opened this issue Jun 3, 2024 · 2 comments
Closed
Labels
question Further information is requested

Comments

@maxsos
Copy link
Contributor

maxsos commented Jun 3, 2024

Description:

Current Implementation:
https://github.com/pydantic/pydantic-extra-types/blob/main/pydantic_extra_types/color.py#L109

The current code uses a try-except block to handle a potential KeyError when looking up a color value in the COLORS_BY_VALUE dictionary:

try:
    return COLORS_BY_VALUE[rgb]
except KeyError as e:
    if fallback:
        return self.as_hex()
    else:
        raise ValueError('no named color found, use fallback=True, as_hex() or as_rgb()') from e

Proposed Change:
Refactor the code to use an if-else conditional check instead of a try-except block. This change improves code readability and leverages a more predictable conditional flow:

if rgb in COLORS_BY_VALUE:
    return COLORS_BY_VALUE[rgb]
else:
    if fallback:
        return self.as_hex()
    else:
        raise ValueError('no named color found, use fallback=True, as_hex() or as_rgb()')

Benefits:

  • Enhances code readability by removing the exception handling for a simple key lookup.
  • Clearly separates the logic for handling the presence and absence of the key in the dictionary.

Impact:
This change is expected to maintain the existing functionality while improving code clarity.

Please let me know if there are any specific tests or cases you would like me to address in this refactoring.

Thank you!

@yezz123 yezz123 added the question Further information is requested label Jun 22, 2024
@yezz123
Copy link
Collaborator

yezz123 commented Jun 22, 2024

Hey @maxsos Sorry for the late response 🙏🏻

Feel free to open a PR, and we can discuss the potential behavior changes and any specific tests or cases that need to be addressed.

@yezz123
Copy link
Collaborator

yezz123 commented Jun 23, 2024

fixed #192

@yezz123 yezz123 closed this as completed Jun 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants