search

OCTRI / fhir2hpo

Library for converting FHIR messages to HPO
MIT License
7 stars 2 forks source link

Combine classes that code internal FHIR and other codes like SNOMED #8

Closed aeyates closed 6 years ago

aeyates commented 6 years ago

@kingmanzhang Great work on adding the concept of systems to support SNOMED. I made some improvements to combine the "CodedValue" and the "Loinc2HpoCodedValue" into a single class tentatively named "HpoEncodedValue". This make things a bit cleaner so no adapter is needed. If this looks good, we can merge with your branch and then merge your branch into master.

kingmanzhang commented 6 years ago

Looks good to me and passed all tests on my machine! I like the way you combined enum class to HpoEncodedValue. That is smart! The only hesitation I have is the name of the new class--HpoEncodedValue. We can use it for now, but eventually we may change it to a more general name like "CodedOutcome" because it will be used to represent internal codes AND external codes (e.g. a snomed concept, or any categorical value). You can go ahead and merge.

aeyates commented 6 years ago

Thanks, @kingmanzhang. I wasn't settled on the name of that class either - it was just easier to keep it distinct while making the changes. But I was thinking we might generalize it to CodedValue in the long run.