There is a long standing ticket to fix this and just remove empty attributes. I can't remember why I voted against doing it easy back when. I'd link to the ticket but I'm on my phone...I think Arbaugh opened it a few years ago.
Ben On Sep 26, 2011 9:10 PM, "Dave Thomas" <[email protected]> wrote: > sorry to jump into this discussion late, and maybe with a lame, > already-covered comment. > > i would say that maybe 10-15% of our sync traffic are > patientAttributes with value '''. (empty string), which are totally > meaningless rows that are basically just feeding MTN money for > bandwidth. It would be nice if in the design process we could > minimize these. Do we even really need attribute rows where value = > NULL?, or can we at least get rid of the ones that really don't mean > anything? > > d > > On Sun, Sep 25, 2011 at 8:18 PM, Darius Jazayeri > <[email protected]> wrote: >> Multiplicity constraints on attributes are enforced by the validator of the >> class that owns those attributes. I.e. if you have a VisitAttributeType with >> minOccurs=1 and maxOccurs=3, that will be checked when you validate the >> Visit it belongs to. >> I'd hope things would work like this: >> (1) visit.setAttribute(attrType, null) --> semantically equivalent to "void >> any existing attributes of the given type" >> (2) visit.addAttribute(attrType, null) --> this should fail when you try to >> save the visit, because a null-valued attribute is invalid. (The db has a >> not-null constraint on the value column.) To get a pretty error in this >> case, we'd probably need to handle this in the controller for the edit visit >> page. But at least the AOP would prevent us from storing empty values for >> attributes (unintentionally sidestepping the not-null constraint.) >> -Darius >> On Sun, Sep 25, 2011 at 8:09 AM, Friedman, Roger (CDC/CGH/DGHA) (CTR) >> <[email protected]> wrote: >>> >>> I actually agree that something should be put in the field, but I have >>> seen so many variants (blank, ?, ??, ???, Unknown, Uknown, Unown, Not Known, >>> Not Available, Missing, Left Card At Home) that I have come to believe that >>> the easiest thing to say to users is “if you don’t know it, leave it >>> blank”. The form controller normally will know whether an unknown value is >>> required and if so what its representation should be; however, that doesn’t >>> apply to our 3 formentry modules. In the case of OpenMRS objects, we could >>> use ID=-1 to indicate unknown, perhaps providing a default entry in the >>> underlying table (to permit inner rather than left joins). >>> >>> >>> >>> But you still haven’t said where the multiplicity constraints on >>> attributes should be enforced and whether that’s compatible with using AOP >>> to convert empty strings to nulls and what the effect of updating an >>> attribute value to null should be. >>> >>> >>> >>> From: [email protected] [mailto:[email protected]] On Behalf Of Darius >>> Jazayeri >>> Sent: Friday, September 23, 2011 2:44 PM >>> >>> To: [email protected] >>> Subject: Re: [OPENMRS-DEV] Empty string fields vs null fields >>> >>> >>> >>> I agree with Burke here, I think that allowing people to store "" in a >>> required field, and using that to represent "yes, but unknown" isn't right. >>> You should store an explicit and recognizable unknown value, e.g. "Unknown". >>> So for the guardian case, no guardian means do not store a row in the >>> person_attribute table for the non-existent guardian. And an >>> existent-but-unknown guardian would be stored as an attribute whose value is >>> a pointer to an "Unknown Person" person. >>> >>> >>> >>> Also, at a web UI level, the standard widgets don't distinguish between >>> these things (i.e. if you have an attribute type whose datatype is "person", >>> the out-of-the-box widget is just going to show a person chooser, not have >>> an extra "click for unknown")... >>> >>> >>> >>> -Darius >>> >>> On Friday, September 23, 2011, Burke Mamlin wrote: >>> >>> Roger, >>> >>> >>> >>> As you suggest, treating all empty strings as null will enforce that >>> required fields cannot be blank, since we won't be able to distinguish >>> between answered with an empty string from not answered at all. That said, >>> when is it "okay" to respond to a required question with nothing? Wouldn't >>> your example be better handled by recording the "Unknown" (in the case that >>> the orphan has a legal guardian who is currently unknown) instead of >>> treating unknown the same as if the field were skipped? And/or record "N/A" >>> when the field doesn't apply (in OpenMRS, we would probably just omit >>> storing an observation altogether if it didn't apply). >>> >>> >>> >>> -Burke >>> >>> On Fri, Sep 23, 2011 at 8:59 AM, Friedman, Roger (CDC/CGH/DGHA) (CTR) >>> <[email protected]> wrote: >>> >>> Darius – >>> >>> I believe I gave several examples during the earlier round of this >>> discussion. Here’s one – an orphan has a legal guardian, but the person >>> bringing the orphan to the clinic doesn’t know who, so the field is left >>> blank; a non-orphan does not have a legal guardian. So when it comes time >>> to report, you do something patient left join person_attribute where >>> attribute_type=”Legal Guardian” and format the nulls as “N/A” and print the >>> empty strings. >>> >>> I don’t think the automagic works with the new attribute paradigm. >>> Take a required attribute like National ID. The patient does have her card >>> with her when she comes to the clinic. Now we can’t save the patient info. >>> So I withdraw my tentative conclusion about properties. Is the >>> implementation of the minimum constraint still undecided? Let’s make sure >>> that’s well-understood before moving on to automagic. >>> >>> >>> >>> From: [email protected] [mailto:[email protected]] On Behalf Of Darius >>> Jazayeri >>> Sent: Thursday, September 22, 2011 4:36 PM >>> >>> To: [email protected] >>> Subject: Re: [OPENMRS-DEV] Empty string fields vs null fields >>> >>> >>> >>> https://tickets.openmrs.org/browse/TRUNK-2680 >>> >>> On Thu, Sep 22, 2011 at 1:24 PM, Darius Jazayeri < [email protected]> >>> wrote: >>> >>> There is already AOP happening on every XyzService.saveXyz call. So we're >>> actually already paying the significant overhead. I think that >>> reflection+iterating over properties only on saveXyz methods is vanishingly >>> small additional overhead to the fact that every service call is AOPd. >>> >>> >>> >>> -Darius >>> >>> >>> >>> On Thu, Sep 22, 2011 at 12:51 PM, Burke Mamlin <[email protected]> >>> wrote: >>> >>> On Thu, Sep 22, 2011 at 3:03 PM, Darius Jazayeri < [email protected]> >>> wrote: >>> >>> So, does that mean you're fine with automagically changing empty String >>> properties of OpenmrsObjects to null before saving to the database? >>> >>> >>> >>> Yes. I love magic (I'm no muggle!)… as long as it's not going to add >>> several milliseconds or more to every API call. If doing it automagically >>> involves AOP or some other hook that is going to add (or contribute to) a >>> measurable performance hit, I would rather find a way to manage it >>> explicitly (via hibernate configuration, a custom hibernate type, and/or >>> code reviews). >>> >>> >>> >>> -Burke >>> >>> >>> >>> ________________________________ >>> >>> Click here to unsubscribe from OpenMRS Developers' mailing list >>> >>> ________________________________ >>> >>> Click here to unsubscribe from OpenMRS Developers' mailing list >>> >>> ________________________________ >>> Click here to unsubscribe from OpenMRS Developers' mailing list >> >> ________________________________ >> Click here to unsubscribe from OpenMRS Developers' mailing list > > _________________________________________ > > To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. > > [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l] _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
