You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This issue is mainly intended to gather feedback from users we are potentially not yet aware of. During the discussion at the EDM4hep meeting on Aug 10 there was general agreement that the following proposal should be adopted. However, we also agreed that we would still like to collect more feedback as it constitutes a breaking change. Additionally, it should also help us with possible migration strategies.
Please let us know what you think
TL;DR (reasoning below)
The proposal is to make the Const user-facing classes that are generated by podio the "default" classes and rename the current "default" classes to something that explicitly marks them as mutable. Either via
putting these classes into a separate namespace (e.g. mut or mod)
or by sticking a Mutable (or Mut) to the class name as pre- or post-fix
The main reason for this proposal is that it makes the default use case of simply reading data easier to write, and additionally making it very clear when something needs to be mutated. The details of the naming scheme are still part of the discussion and input is welcome.
Current situation in podio
For the example datatype
MCParticle:
Members:
- int pdg // PDG
podio generates the following user classes at the moment
The only difference between the two classes is that the Const version offers only getter functions, but no setter functions. The collection interface looks something like this (just showing enough to make the point)
Again a duplication of the classes is necessary here to make the interface const-correct; Depending on whether the collection is const (in the c++ sense) access to the Const (in the podio sense) or the "normal" classes is granted.
Problems / Issues with the current approach
The presence of the Const classes is not documented (Document Const* Interfaces #198) even though they should be the default use case for reading
The default use case should be the "easier one to use"
There is no implicit conversion from the "default" class to the Const class. This is by design as these conversions should only make the access more strict.
Where users might run into problems with the current approach is when writing interfaces for podio generated EDMs, e.g.
voidfoo(const MCParticle& p); // const-ref is a slightly unusual construct with value semanticsvoidbar(MCParticle p); // One would expect that this worksconstauto& coll = store.get<MCParticleCollection>("mcs");
auto p = coll[0];
foo(p); // error: invalid initialization of reference of type 'const MCParticle&' from expression of// type 'ConstMCParticle'bar(p); // error: could not convert 'p' from 'ConstMCParticle' to 'MCParticle'
Here users actually need to know about the presence of the Const in order to write interfaces that only need reading access. This makes it unnecessarily complicated to write an interface. It would become simpler if the "default" classes are immutable and mutable access requires "more work".
The text was updated successfully, but these errors were encountered:
This issue is mainly intended to gather feedback from users we are potentially not yet aware of. During the discussion at the EDM4hep meeting on Aug 10 there was general agreement that the following proposal should be adopted. However, we also agreed that we would still like to collect more feedback as it constitutes a breaking change. Additionally, it should also help us with possible migration strategies.
Please let us know what you think
TL;DR (reasoning below)
The proposal is to make the
Const
user-facing classes that are generated by podio the "default" classes and rename the current "default" classes to something that explicitly marks them as mutable. Either viamut
ormod
)Mutable
(orMut
) to the class name as pre- or post-fixThe main reason for this proposal is that it makes the default use case of simply reading data easier to write, and additionally making it very clear when something needs to be mutated. The details of the naming scheme are still part of the discussion and input is welcome.
Current situation in podio
For the example datatype
podio generates the following user classes at the moment
The only difference between the two classes is that the
Const
version offers only getter functions, but no setter functions. The collection interface looks something like this (just showing enough to make the point)Again a duplication of the classes is necessary here to make the interface const-correct; Depending on whether the collection is
const
(in the c++ sense) access to theConst
(in the podio sense) or the "normal" classes is granted.Problems / Issues with the current approach
Const
classes is not documented (DocumentConst
* Interfaces #198) even though they should be the default use case for readingConst
class. This is by design as these conversions should only make the access more strict.Where users might run into problems with the current approach is when writing interfaces for podio generated EDMs, e.g.
Here users actually need to know about the presence of the
Const
in order to write interfaces that only need reading access. This makes it unnecessarily complicated to write an interface. It would become simpler if the "default" classes are immutable and mutable access requires "more work".The text was updated successfully, but these errors were encountered: