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

Streamline primitive's Value, ObjecValue and "original text" #2781

Open
ewoutkramer opened this issue May 3, 2024 · 0 comments
Open

Streamline primitive's Value, ObjecValue and "original text" #2781

ewoutkramer opened this issue May 3, 2024 · 0 comments
Labels
breaking change This issue/commit causes a breaking change, and requires a major version upgrade

Comments

@ewoutkramer
Copy link
Member

The Primitive POCO's currently have an internal representation that is the same as the type of their Value property. They are listed in the IReadOnlyDictionary documentation.

POCO's also have an ObjectValue property, that should contain their internal representation (so the same as Value), but can also be used to store a verbatim string (from the json or xml) if it could not be parsed into the format required by the Value (i.e. an integer in Integer that was unparseable in the XML attribute).

For some operations on these primitives, the internal representation is not good enough, i.e. FhirDateTime needs to parse the string value in it to do comparisons. Conversely, some of the constructors of FhirDateTime pass in a discrete date time (already parsed), but since it stores a string internally, these are formatted to a string and then thrown away.

The current setup has a few disadvantages:

  • There is unnecessary formatting/parsing going on for many types
  • It is unclear what ObjectValue will contain, as its type is object and it can both contain a valid Value or text. I often have to refer back to our documentation to figure out what could be in ObjectValue.
  • We do not keep the exact text of the source (json/xml, whatever) for roundtripping, we only do that for inparseable data.

We should fix these disadvantages, maybe like so:

  • Make ObjectValue a readonly property or add ToObjectValue() on PrimitiveType.
  • Have a "OriginalText" property that is set by the parser/raw text constructor and gets erased when its overwritten via its POCO interface. Ideally this would be read-only, and can be set by an explicit Parse() on the datatype, but this would require the new static interface members (Parse) on a IPrimitiveType or somesuch.
  • Keep the Value property as is
  • Add SetValue(string originalText, object parsed) - if we want to allow different parsers/sources to be able to supply unparsed/parsed values into the primitive, so it does not have to be reserialized if an underlying format already has a usable form.
@ewoutkramer ewoutkramer added enhancement breaking change This issue/commit causes a breaking change, and requires a major version upgrade labels May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This issue/commit causes a breaking change, and requires a major version upgrade
Projects
None yet
Development

No branches or pull requests

2 participants