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

use a context for the _resolve_*() methods #290

Closed
andlaus opened this issue Apr 16, 2024 · 1 comment
Closed

use a context for the _resolve_*() methods #290

andlaus opened this issue Apr 16, 2024 · 1 comment
Labels
enhancement New feature or request fringe Tangentially related stuff, e.g., separate projects that can be built using odxtools

Comments

@andlaus
Copy link
Collaborator

andlaus commented Apr 16, 2024

currently most methods for resolving references (resolve_odxlinks() and _resolve_snrefs()) take the odxlinks database or the diagnostic layer as their argument. Unfortunately, this is not always sufficient because some of these methods require additional parameters (example). The approach currently taken is to introduce new _resolve_*() methods for affected classes which must be called instead of the generic versions. Alternatively it would be possible to pass an object describing the full calling context and let the called method determine the required information. E.g.

def _resolve_snrefs(self, context: CallingContext) -> None:
    params = context.parameters
    ...

Note that this would require a major refactoring because all classes would be affected and it might even make the code worse (in the sense of "more complicated")

@andlaus andlaus added enhancement New feature or request fringe Tangentially related stuff, e.g., separate projects that can be built using odxtools labels Apr 16, 2024
@andlaus
Copy link
Collaborator Author

andlaus commented Oct 30, 2024

this has been implemented for _resolve_snref() by means of the SnrefContext class. for _resolve_odxlinks() I came to the conclusion that this is not necessary and would just bloat the code. (Note that we cannot consolidate these two methods into one because before SNREFs can be resolved, all ODXLINK references and value inheritance needs to have been dealt with.)

@andlaus andlaus closed this as completed Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fringe Tangentially related stuff, e.g., separate projects that can be built using odxtools
Projects
None yet
Development

No branches or pull requests

1 participant