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 PythonConstant types into a true union #234

Merged
merged 1 commit into from
Oct 6, 2024

Conversation

atifaziz
Copy link
Contributor

@atifaziz atifaziz commented Oct 6, 2024

This PR proposes to refactor PythonConstant types into a true union via a class hierarchy where PythonConstant becomes an abstract base class and the various types its nested subclasses. This has the following benefits:

  • Each subclass has its own clear, simpler and overriding implementation of ToString instead of a switch-case.
  • Each subclass holds its own strong-typed value, e.g. Value on PythonConstant.Integer is typed as long whereas Value on PythonConstant.String is typed as string.
  • Each subclass only requires the storage space for its value instead of the sum of all possible values.
  • Subclasses like Bool and None have statically initialised values, which avoids unnecessary allocations.
  • PythonConstant.ConstantType is no longer needed.
  • Subclasses enable simpler, stronger and more robust pattern-matching (see updates in ArgumentReflection.ArgumentSyntax).

@tonybaloney
Copy link
Owner

Thanks, this is a lot cleaner. I think we started with 2 constant types then it got out of hand quickly

@tonybaloney tonybaloney merged commit 022e6fa into tonybaloney:main Oct 6, 2024
37 checks passed
@atifaziz atifaziz deleted the pyconst-union branch October 7, 2024 06:08
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