-
Notifications
You must be signed in to change notification settings - Fork 0
Add FHIR Type to fromFHIR function signature #46
Changes from 5 commits
0b76561
bcd3d87
cb179e7
3b528c6
71b92ba
574c7fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,15 +41,16 @@ function generateFactory(namespaces) { | |
cw.blComment(() => { | ||
cw.ln('Create an instance of a class from its FHIR representation.') | ||
.ln('@param {Object} fhir - The element data in FHIR format (use `{}` and provide `type` for a blank instance)') | ||
.ln(`@param {string} [type] - The (optional) type of the element (e.g., 'http://standardhealthrecord.org/spec/shr/demographics/PersonOfRecord'). This is only used if the type cannot be extracted from the JSON.`) | ||
.ln(`@param {string} fhirType - the type of the FHIR object that was passed in, in case not otherwise identifiable from the object itself`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is Also note that in jsdoc, you can indicate a parameter is optional by surrounding it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I see you did use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think I first learned I could do that between here and the namespace factory. For consistency I'll change the namespace factory comment to remove the brackets. Technically whether it's optional or not will depend on the individual class, since most classes won't use it but some will require it. |
||
.ln(`@param {string} shrType - The (optional) type of the element (e.g., 'http://standardhealthrecord.org/spec/shr/demographics/PersonOfRecord'). This is only used if the type cannot be extracted from the JSON.`) | ||
.ln('@returns {Object} An instance of the requested class populated with the provided data'); | ||
}) | ||
.bl('static createInstanceFromFHIR(fhir, type, shrId=uuid(), allEntries=[], mappedResources={}, referencesOut=[], asExtension=false)', () => { | ||
cw.ln('const { namespace } = getNamespaceAndNameFromFHIR(fhir, type);') | ||
.bl('static createInstanceFromFHIR(fhir, fhirType, shrType, shrId=uuid(), allEntries=[], mappedResources={}, referencesOut=[], asExtension=false)', () => { | ||
cw.ln('const { namespace } = getNamespaceAndNameFromFHIR(fhir, shrType);') | ||
.ln('switch (namespace) {'); | ||
for (const ns of namespaces) { | ||
const factory = factoryName(ns.namespace); | ||
cw.ln(`case '${ns.namespace}': return ${factory}.createInstanceFromFHIR(fhir, type, shrId, allEntries, mappedResources, referencesOut, asExtension);`); | ||
cw.ln(`case '${ns.namespace}': return ${factory}.createInstanceFromFHIR(fhir, fhirType, shrType, shrId, allEntries, mappedResources, referencesOut, asExtension);`); | ||
} | ||
cw.ln(`case 'primitive': return fhir;`); | ||
cw.ln(`default: throw new Error(\`Unsupported namespace: \${namespace}\`);`) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -194,20 +194,20 @@ export const FHIRHelper = { | |
|
||
/** | ||
* Creates an ES6 class instance based on a value extracted from the JSON. | ||
* @param {string} key - the original key under which the value was stored. This is used as a backup in case the value | ||
* does not declare its type. | ||
* @param {object} value - the FHIR data to create an ES6 class instance for | ||
* @param {string} shrType - the fqn of the class to be instantiated | ||
* @param {object} fhir - the FHIR data to create an ES6 class instance for | ||
* @param {string} fhirType - the type of the FHIR object passed in, just in case it's not otherwise available by inspecting the object | ||
* @returns {object} an instance of an ES6 class representing the data | ||
* @private | ||
*/ | ||
createInstanceFromFHIR: function(key, value, shrId, allEntries=[], mappedResources={}, referencesOut=[], asExtension=false) { | ||
if (Array.isArray(value)) { | ||
return value.map(v => FHIRHelper.createInstanceFromFHIR(key, v, shrId, allEntries, mappedResources, referencesOut, asExtension)); | ||
createInstanceFromFHIR: function(shrType, fhir, fhirType, shrId, allEntries=[], mappedResources={}, referencesOut=[], asExtension=false) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code has three basic implementations of functions called The latter two have signature:
But this one has signature:
Note that they are almost exactly the same except for the order of the first three arguments. Ideally, there should be consistency between all of them in order to avoid stupid and unintentional mistakes (probably to be made by me). So... can we either change this signature to match the others or change the others to match this one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated the other locations to match this one, I think it's fewer code changes and in simpler places, plus the ordering feels more consistent with how |
||
if (Array.isArray(fhir)) { | ||
return fhir.map(v => FHIRHelper.createInstanceFromFHIR(shrType, v, fhirType, shrId, allEntries, mappedResources, referencesOut, asExtension)); | ||
} | ||
if (OBJECT_FACTORY == null) { | ||
throw new Error(`SHR ES6 module is not initialized. Import 'init' before using the ES6 factories and classes`); | ||
} | ||
return OBJECT_FACTORY.createInstanceFromFHIR(value, key, shrId, allEntries, mappedResources, referencesOut, asExtension); | ||
return OBJECT_FACTORY.createInstanceFromFHIR(fhir, fhirType, shrType, shrId, allEntries, mappedResources, referencesOut, asExtension); | ||
}, | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{ | ||
"resourceType": "Observation", | ||
"id": "0000-0000", | ||
"subject": { "reference": "abcd-1234" }, | ||
"valueCodeableConcept": { | ||
"coding": [{ | ||
"system": "SNOMED-CT", | ||
"code": "1234567890", | ||
"display": "Dummy SNOMED Code" | ||
}] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"resourceType": "Observation", | ||
"id": "0000-0000", | ||
"subject": { "reference": "abcd-1234" }, | ||
"valueString": "something or other" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to assume that the type at index
0
is the right type?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. Off the top of my head I'm not sure of all the reasons that a FHIR element might have multiple types, but if this was originally a choice type like
value[x]
then it's been pre-processed so that it appears to be a single choice with a single type, and that's the use case where this is most relevant.Not conclusive proof, but running through the current spec there are no instances where there is a second type available to choose here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Yes, if we've already handled
[x]
types before this then I think that should be fine.