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 AbstractExpressionConverter convert/init behavior #4

Closed
KarlEilebrecht opened this issue Jan 6, 2025 · 1 comment
Closed
Assignees
Labels
enhancement New feature or request

Comments

@KarlEilebrecht
Copy link
Owner

KarlEilebrecht commented Jan 6, 2025

The AbstractExpressionConverter has an init(rootContext)-method, a template method intended to be overridden by
subclasses and a final convert(expression)-method to seal the usage pattern of all converters.

The idea was that converters can be initialized with an initial context that differs from the one that is provided by the context supplier, and converters should be reusable.

Problem

The current design has a logical error that leads to weird behavior:

  • The first thing the convert(..)-method does is clearing the root-context, which does not make sense if you want to prepare a context before use.
  • The convert-method does not call the init(..)-method.
  • To properly re-use a converter you MUST call init(..) to avoid side-effects (artifacts from the previous conversion).

Proposed improvement

  • Keep the convert(..)-method final, but let it call the init(..)-method with the existing context so that this method can decide how to clean or reset or replace the root-context.
  • Don't clean the root-context in the convert(..)-method.

This way, subclasses have better control over the state management of the converter and the usage pattern of the convert(..)-method gets clarified: no two runs get intertwined anymore.

@KarlEilebrecht
Copy link
Owner Author

Reworked the state management entirely. It is now possible to intercept the context initialization on demand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant