Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_base
-
Labels:None
-
Story Points:8
-
Epic Link:
-
Sprint:DRP S17-3, DRP S17-4
-
Team:Data Release Production
Description
As discovered by DM-6561, the FlagHandler mechanism for connecting the enumeration of flags in C++ and the order in which the flags appear in the schema and internal FlagHandler structures is not robust. Fix this, problem, so that the identifier used identify a particular flag and the lookup of the Flag Key are guaranteed to match.
Then fix the flagDecorator (it will be simpler) and all of the algorithms to match the new FlagHandler scheme.
Attachments
Issue Links
Activity
About your suggestion that the add methods could return FlagDefinition const &, I didn't think that this would be safe with the data stored in an underlying std::vector<FlagDefinition>. At one point, I was writing my own storage class to fix this problem, but Jim asked me not to. Can you think of a way to make this safe? I definitely saw problems with the std::vector when I used this class from Python.
Perry Gee
added a comment - Bob Armstrong
About your suggestion that the add methods could return FlagDefinition const &, I didn't think that this would be safe with the data stored in an underlying std::vector<FlagDefinition>. At one point, I was writing my own storage class to fix this problem, but Jim asked me not to. Can you think of a way to make this safe? I definitely saw problems with the std::vector when I used this class from Python. I'm don't think was my suggestion. I was actually talking about the arguments to the function.
But as to your question, I think you were having problems because you are adding elements to the vector via push_back which would invalidate the references? Not because of the underlying storage?
Bob Armstrong Yes, I was adding elements. I wasn't expecting it to be stable. I was just saying that there would be an advantage to using a storage class with underlying stable memory allocation, as pointers to array members would then be stable. It was a small thing. Making a copy seems reasonable in this case.
Perry Gee
added a comment - Bob Armstrong Yes, I was adding elements. I wasn't expecting it to be stable. I was just saying that there would be an advantage to using a storage class with underlying stable memory allocation, as pointers to array members would then be stable. It was a small thing. Making a copy seems reasonable in this case. 
I looked through all the code, but mostly focused on meas_base. It looks good. There are only a few minor comments that need to be addressed on the pull request.