GEOS icon indicating copy to clipboard operation
GEOS copied to clipboard

Rename `extrinsicData` into `field`

Open TotoGaz opened this issue 1 year ago • 4 comments

We now extensively use extrinsicData when registering information here and there (some code seems duplicated in ObjectManagerBase and ConstitutiveBase... whatever). It's proposed to use the fields naming instead. This should bring shorter variable and function names. The concept associated with the naming should be more straightforward to grasp too.

Among the changes which I thing will spark no flame war, we have

  • renaming namespace extrinsicMeshData into namespace fields
  • renaming getExtrinsicData into getField
  • renaming hasExtrinsicData into hasField

Now, we also have the registerExtrinsicData which takes a list of tuples. As such, I think we should rename it registerFields with an s. Do we agree?

The macro that currently defines the traits is called EXTRINSIC_MESH_DATA_TRAIT and should be renamed as well. The new name is not obvious. But I think we should avoid having TRAITS in the name as well. Do we want something like

  • DECLARE_FIELD
  • DEFINE_FIELD
  • DESCRIBE_FIELD
  • SPECIFY_FIELD
  • else?

To be sure there is no misunderstanding between the declaration and the registration, we could add the _INFO suffix?

  • DECLARE_FIELD_INFO
  • DEFINE_FIELD_INFO
  • DESCRIBE_FIELD_INFO
  • SPECIFY_FIELD_INFO
  • else?_INFO

Some files names contain ExtrinsicData. Like MultiFluidExtrinsicData.hpp. Consistently, I should be renaming them

  • MultiFluidFieldDeclarations.hpp
  • MultiFluidFieldDefinitions.hpp
  • MultiFluidFieldDescriptions.hpp
  • MultiFluidFieldSpecifications.hpp

What's your naming choice? Anything I forgot? (Some documentation needs to be updated obviously)

TotoGaz avatar Oct 18 '22 19:10 TotoGaz

Thanks for taking care of that. My preferences go to the shortest options:

  • namespace extrinsicMeshData -> namespace fields
  • getExtrinsicData -> getField
  • hasExtrinsicData -> hasField
  • EXTRINSIC_MESH_DATA_TRAIT -> DECLARE_FIELD
  • MultiFluidExtrinsicData.hpp -> MultiFluidFields.hpp

If you want, in the cpp files of the flow solvers, you can also implement this comment from @rrsettgast and put the using namespace fields at the beginning of the files to have even shorter names, it is really easy to do.

francoishamon avatar Oct 18 '22 21:10 francoishamon

Thx @francoishamon for your comments. Any preference for the register* family? Do you want an s? 😉

If you want, in the cpp files of the flow solvers, you can also implement https://github.com/GEOSX/GEOSX/pull/2098#discussion_r985714377 from @rrsettgast and put the using namespace fields at the beginning of the files to have even shorter names, it is really easy to do.

While surely straightforward, I will not do it this because I can just fail, get unexpected name collisions, etc. We can surely do it after this PR to see the result (with simple fields, maybe we'll realize we're happy as it is).

TotoGaz avatar Oct 18 '22 21:10 TotoGaz

Maybe

  • registerExtrinsicData -> registerField for the function that takes one trait template
  • registerExtrinsicData -> registerFields for the function that takes multiple trait templates

?

francoishamon avatar Oct 19 '22 00:10 francoishamon

If it's possible I'll do that. I don't know how this will behave w.r.t. the recursive templated calls, but it may be OK! 🤞

TotoGaz avatar Oct 19 '22 01:10 TotoGaz