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

Clean public API of components: towards more explicit delegation #5020

Open
hugtalbot opened this issue Sep 25, 2024 · 2 comments
Open

Clean public API of components: towards more explicit delegation #5020

hugtalbot opened this issue Sep 25, 2024 · 2 comments
Assignees
Labels
issue: discussion Open topic of discussion STC#18 Tasks for STC#18 coding sprint

Comments

@hugtalbot
Copy link
Contributor

hugtalbot commented Sep 25, 2024

This issue is created further to #4943

This PR proposed to define base class functions as final and override these functions (which are templated) in child classes as a protected function. These changes are breaking but could be done in several steps:

  1. set the function of the abstract class to final
  2. fix the components which were overriding this function [FEM] Components override the template API instead of the generic one #4982
  3. set the functions overriding the templated one as protected (for v25.06 ?)
  4. rename functions with a clear delegate name doFunctionName() + compatibility layer
  5. tests should be fixed to use the public virtual class (access protected functions)
  6. add the check on the component state:
    • first check non-invalidity
    • then, check the validity

This project could have several benefits:

  • cleaning the API and expliciting the delegation mechanism
  • allowing to introduce and generalize more easily the componentState mechanims
@damienmarchal
Copy link
Contributor

nice

@hugtalbot hugtalbot added STC#18 Tasks for STC#18 coding sprint issue: discussion Open topic of discussion labels Sep 25, 2024
@alxbilger
Copy link
Contributor

If we do that, I recommend to use strong types in the functions accepting VecId. Indeed, today the VecId for a force is the same type than the VecId for a velocity, and it's a source of mistake when the function accepts both forces and velocities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: discussion Open topic of discussion STC#18 Tasks for STC#18 coding sprint
Projects
None yet
Development

No branches or pull requests

4 participants