[WIP] Add posibility to register external code lists.

[RomanKapitonov]

Addresses #161

Below is a sample implementation as a simple proof of concept, which may definitely be extended further. I would like to initiate a discussion on the pros/cons of the suggested approach.

@irobayna @kputnam Guys, I would like to know your thoughts on this.

[irobayna]

@RomanKapitonov WOW, this is fantastic! This will be one of those key features will be very handy to have.

I have to take a deeper dive and study your PR but out of the gate, I see that the new logic returns an empty if the code list does not exist. What do you think about perhaps having some way to alert that the code list does not exist? I could foresee someone making a typo and perhaps not realizing this or worse yet getting the wrong code list.

Will take a deeper dive and report back.

Isi

[RomanKapitonov]

@irobayna Thank you! I'm planning to continue working on the PR sometime this week. I've actually added a warning upon referencing a non-existing external list but then decided to get rid of it. I'm going to add it back along with documentation and some examples of usage.

[RomanKapitonov]

@irobayna I've been looking through some of the specs for 270/271/837I and others, and it looks like some of the code lists might have "sub-lists". This concept is also covered by stupidedi. For example, the definition of DMG segment might contain element DMG10 which is data element 1270. Based on how 1270 is defined it looks like the element may contain a reference to the external code list. But that doesn't make much sense to me.

How would it be used? I mean let's say a DMG segment has some value in DMG10, which in terms is a reference to the external code list:

E1270 = t::ID.new(:E1270, "Code List Qualifier Code"             , 1, 3,
          s::CodeList.build(
            "65"  => s::CodeList.external("508"),
            "68"  => s::CodeList.external("682"),
            "AAA" => s::CodeList.external("662"),
            ...

Say we have a DMG:

DMG**********65~

Then how will code list "65" be used?

[kputnam]

Hey @RomanKapitonov, this looks promising! I really like the ability to have the lookup values in the pretty printed output. I also agree with removing the AbstractElementUse#code_lists method, since it's not called anywhere (except a few other methods that aren't used).

One thing I'm not sold on is having a mutable global variable, Stupidedi.external_code_lists. I think we can have this data be a member of Config instead, by using or extending Config#code_lists. It's currently just waiting to be used for future work like this. The type (CodeListConfig) seems similar to ExternalCodeListStorage.

I would like to use an approach that allows different files to use different external code list implementations (e.g., maybe it depends on who the sender or receiver is), without the two files interfering with each other's external code list table. That also reduces the risk that code in an unrelated part of your application (or loaded via a third-party library) could change that global variable; if instead it's part of Config you would need to pass the config explicitly for that to happen. I think you also noticed that potential issue, because you have register print a warning when replacing an existing code list.

My first thought was to pass the config around to each method as it's needed. It is conveniently stored next to the parse tree in StateMachine#config. But passing arguments to IdentifierVal#inspect is unconventional, and even difficult to accomplish when it's called from q.pp(...) in one of the pretty_print methods. Similarly,SimpleElementVal#allowed? would require an argument, because it needs the code list to check.

Looking at the bigger picture, that would place the process of linking an element to an external code list as late as possible -- only right when we need to check its membership or pretty print it. However, we have all the info needed when we are constructing the element value (i.e., this is E26 so we know we should use config.code_list.at("5") or whatever), so we could link the element to the external code list as soon as we're constructing it. That brings me to this plan...:

Normally SimpleElementVal is given its usage attribute in AbstractState.mksegment; it eventually gets to element_use.value(element_tok.value, element_tok.position), which eventually resolves to IdentifierVal.value(...) for instance, where that's a subclass of SimpleElementVal. That usage attribute is used to get the internal code list for pretty printing and to determine the set of allowed_values.

Instead, I think we can pass an updated element_use in AbstractState.mksegment. The new element_use can be constructed by calling element_use.copy(...) with a new value for definition, which itself is created by updating the existing element_use.definition with a new code_list. We probably also need to update element_use.allowed_values to point to that same code_list too. I think that would tie all the bits together without requiring a lot of surgery.

Side note: we already do something similar in ID.simple_use when an internal code list is used, the difference being that it's allowed_values (which can be further restricted when a user declares a transaction set), and we can do it right when the grammar is being defined since it doesn't depend on a config.

Now one important unresolved issue remains: when one element determines which code list applies to another element. That's what is going on with DMG10. I believe it determines which external code list applies to DMG11. I'm certain that NM108 is used to determine which external code list applies to NM109 -- in the case of HC837, "XX" means there is a Medicare/Medicaid National Provider Identifier (NPI) in NM109. These relationships between the two elements aren't currently described in the SegmentDef schema, but I think that's where they need to go. The relationship is predetermined, it doesn't depend on where the segment is being used.

In summary, eventually AbstractState.mksegment will need to handle two cases:

1. The element definition has a single CodeList.external (e.g., E26, used in DMG07 and N404).
2. The element definition has a CodeList.internal that maps values to an CodeList.external; those external code lists govern another element in that segment. (e.g., E66, seen in NM108).

In the short term, we can make some progress by updating mksegment to handle the first case. This can be done by (a) first checking that we fall under the first case and (b) updating the element_use via element_use.copy(:definition => element_use.definition.copy(:code_list => ...)) before constructing the element value. Then, you should automatically get error checking in BuilderDsl and with a change to this line, you will have the lookup value in the pretty printer output.

Once that's working I think it wouldn't be much of a leap to create a new data type similar to SyntaxNote which lets you declare that NM108 determines which code list applies to NM109. Then update mksegment to make use of that data and pass it along the same way as above.

Hopefully I haven't taken the fun out of it and it makes sense. This idea might have some problems once you get down to actually implementing it... before typing this up I had at least two ill-conceived ideas that fell apart as I was trying to explain them :-)

[kputnam]

Whoops, I should've posted this comment here as my code review.