google / android-fhir

The Android FHIR SDK is a set of Kotlin libraries for building offline-capable, mobile-first healthcare applications using the HL7® FHIR® standard on Android.
https://google.github.io/android-fhir/
Apache License 2.0
494 stars 294 forks source link

Add code documentation for StructureMapExtractionContext #1391

Open williamito opened 2 years ago

williamito commented 2 years ago

StructureMapExtractionContext does not have any documentation comments. The function type notation for the parameters of structureMapProvider would also benefit from including names, especially for the first String parameter, e.g. (suspend (targetStructureMapUrl: String, worker: IWorkerContext) -> StructureMap?) as it provides the canonical URL for the value specified by the Target Structure Map extension of the questionnaire provided in extract().

One caveat I also noticed is that the Target structure map extension Is actually repeatable with cardinality of 0..*. If multiple Target Structure Maps are set, it appears that the String parameter gives you the last one set. This should also be documented, and may warrant an eventual redesign to support multiple structure maps?

Edit: After more thought, I think the caveat is actually a bug that if multiple Target Structure Maps are set, extract presumably only happens on the last one. This would still be a change to the API, as we'd need to possibly return multiple bundles.

ekigamba commented 2 years ago

One caveat I also noticed is that the Target structure map extension Is actually repeatable with cardinality of 0..*. If multiple Target Structure Maps are set, it appears that the String parameter gives you the last one set. This should also be documented, and may warrant an eventual redesign to support multiple structure maps?

This is correct. Thanks for bringing this up :smile: . I believe that we might have more complicated uses that we might not be supporting and can possibly have an issue to define and implement this when needed. I might have overlooked this during the implementation

StructureMapExtractionContext does not have any documentation comments

This is a mistake. I probably forgot this while refactoring the code and documentation.

The function type notation for the parameters of structureMapProvider would also benefit from including names, especially for the first String parameter, e.g. (suspend (targetStructureMapUrl: String, worker: IWorkerContext) -> StructureMap?) as it provides the canonical URL for the value specified by the Target Structure Map extension of the questionnaire provided in extract().

I agree on this also.

ekigamba commented 2 years ago

@williamito On the API design for $extract while using StructureMap-based extraction, can we get further clarification since the output expectations provided here http://build.fhir.org/ig/HL7/sdc/extraction.html#output-expectations contradict what is documented on this page http://build.fhir.org/ig/HL7/sdc/StructureDefinition-sdc-questionnaire-targetStructureMap.html