Library class design suggestions / feedback

[Nahuel92]

Hi everybody, hope you are doing well!!

I have been using this library for a few months and since then I have been constantly running into difficulties when working with Resource abstractions.

Probably because I don't know how to properly use the library but, at least from my perspective, the library class design is somewhat flawed (apologies if this bothers any of you, it is not my intention to be mean but it is how I feel about it after using it for some time).

I have a collection of resources (canonical and non-canonical) and I need to perform some operations on canonical resources, but there is no proper way to gain access to common attributes for canonical resources.

As an example, StructureDefinition, ValueSet, CodeSystem all of them have a field called identifier. But this field should be present in their parent (DomainResource) rather than repeating that field in each child class. That way, we don't have to cast each object to their specific type and we could simply use, let's say, DomainResource to gain access to that common field (imagine having a collection with several canonical resources, that would be a nightmare to code and maintain).

Same happens with several class methods, they should be generic (by using Java generics) in order to return the specific object type rather than returning the top-level Resource class. As an example, IbmResourceUtils.getResource(File file) should return the specific resource type rather than the generic Resource.

Hopefully this will make sense to you and can be considered to improve the software engineers experience using this library.

Again, apologies if this bothers you but the current library class design gets in my way to write clean and maintainable code.

P.S: Unfortunately, no, I cannot provide code snippets proving how painful are my workarounds (for security reasons).

[lmsurpre]

Hi @Nahuel92 and thank you for the feedback. The com.ibm.fhir.model.resource, com.ibm.fhir.model.type, and com.ibm.fhir.model.type.code packages are generated from the conformance artifacts that are shipped with the FHIR specification. You will see that DomainResource does not have any "identifier" field and there are a lot of subsclasses which have no such element either.

Especially for conformance artifacts like the ones you list, I do agree it would be nice to have a common parent (or at least an interface). I was hoping the specification authors would see the value of something like this as well, but to date they don't have a clean type hierarchy for us to use. Have a read through https://chat.fhir.org/#narrow/stream/179280-fhir.2Finfrastructure-wg/topic/Interfaces.20and.20ancestors.20and.20patterns if you'd like the gory detail.

For better or worse, we've chosen to keep our model as close to the specification as possible. I'm definitely aware that this results in cases where we return abstract supertypes like Resource or Element from some getters in the model (e.g. from Bundle.Entry.getResource()) but I'm not sure how adding generics could help that. Feel free to play with the idea and propose something. We've had similar requests in the past like #746 in case you'd like to read through that.

In case you're not aware, we do have .is and .as helper methods on the model to make it very slightly less painful to downcast as needed:

if (resource.is(DomainResource.class)) {
    resource.as(DomainResource.class).getExtension();
} else ...

Alternatively, I think the pattern matching for instanceof (shipped with Java 17) could make a slight improvement to this code:

if (resource instanceof DomainResource dr) {
    dr.getExtension();
} else ...

We've also toyed with generating profile-specific model classes (or facades) in the past...because if you can restrict the types of a given element like this to a single subtype then your code can be much cleaner. Unfortunately, I have nothing to share from those efforts to date.

[Nahuel92]

Hi @lmsurpre and thank you for your quick answer. Sounds fair, I will try to come up with something then.

[lmsurpre]

Had this thought today while watching the JavaOne keynote:

If we make our model classes java records, then we could use the brand new java record pattern matching / destructuring (previewed in Java 19):

if (resource instanceof DomainResource(var text, var contained, var extensions, var modifierExtensions)) {
    for (Extensions ext : extensions) ...
} else ...

One neat thing about that destructuring pattern is that it can be nested in a null-safe way. For example, to further destructure the text variable into its status and div components, one could do this:

if (resource instanceof DomainResource(
        Narrative(var status, Xhtml div), 
        List<Resource> contained, 
        List<Extension> extensions, 
        List<Extension> modifierExtensions)) {
    if (status == NarrativeStatus.Value.EXTENSIONS) {
        for (var extension : extensions) ...
    }
} else ...

Unfortunately, Java is doing position-based destructuring which means you need to declare each and every component...although there has been discussion of using _ as an unnamed/undeclared variable to represent components that the user doesn't care about but thats not in Java 19...but maybe they'll bring that idea back in future?

Also unfortunate (but necessary), Java record classes cannot extend (they already extend java.lang.Record) or be extended, so this would force us to define and use interfaces everywhere... so DomainResource would actually be an interface and not a record... and thus maybe we couldn't use pattern matching with it.

But I think it would work for non-abstract types (e.g. when dealing with our choice type getters that return Element today).

[Nahuel92]

Hi, thanks for your feedback. Unfortunately, I wasn't able to play around this idea since I was reassigned to a different task. I hope to get back to it in the future and see how I can help. Thanks again!!