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]<mailto:djazayeri%[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]<mailto:[email protected]>> wrote:
On Thu, Sep 22, 2011 at 3:03 PM, Darius Jazayeri
<[email protected]<mailto:djazayeri%[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
-Darius
On Thu, Sep 22, 2011 at 11:43 AM, Burke Mamlin
<[email protected]<mailto:[email protected]>> wrote:
On Thu, Sep 22, 2011 at 9:18 AM, Darius Jazayeri
<[email protected]<mailto:djazayeri%[email protected]>> wrote:
I still think that we should be automagic.
I cannot think of a single use case where you want "" and null to be
distinguishable. And Ben, your worry that "I fear there might be places where a
column is not nullable but the user doesn't want a value there" is exactly why
I don't want to allow "". If the field is required, the whole point is that you
are not supposed to be able to store an empty value.
Burke, Roger, others, can anyone give me an example of a real use case in
OpenMRS where we want null and "" to be semantically different in a database
column?
The only case of "" and null being semantically different would be cases where
an empty string is a valid response and distinguishing "answered with blank"
vs. not answered becomes important. I can't think of a case where an "empty"
response would be important to capture.
-Burke
-Darius
On Thu, Sep 22, 2011 at 5:50 AM, Friedman, Roger (CDC/CGH/DGHA) (CTR)
<[email protected]<mailto:[email protected]>> wrote:
Yes I think this is true.
However, I think I agree with Darius that in the context of attributes,
attempting to create a string-valued property with a null value should result
in no new property being created. This could/should be extended to the empty
string if the distinction between the two is difficult to maintain when moving
from the gui to the api. This could also be extended to saying that attempting
to update a property value to null would be handled the same as voiding the
property.
From: [email protected]<mailto:[email protected]>
[mailto:[email protected]<mailto:[email protected]>] On Behalf Of Ben Wolfe
Sent: Thursday, September 22, 2011 7:54 AM
To:
[email protected]<mailto:[email protected]>
Subject: Re: [OPENMRS-DEV] Empty string fields vs null fields
So our take-away here is to not do automagic but instead tell api users to put
trimming/nulling into either their save methods or their setters?
Ben
On Mon, Sep 12, 2011 at 5:24 PM, Darius Jazayeri
<[email protected]<mailto:djazayeri%[email protected]>> wrote:
Actually we've been treating the fact that when you save a patient, you get a
bunch of person attributes with value=empty-string as a bug. (And it's a bug
that occurs precisely because of this null != empty string behavior. I.e. >95%
of the time the significance of that difference is that someone forgets to
treat a submitted empty string as null in a controller.)
-Darius
On Mon, Sep 12, 2011 at 5:56 AM, Friedman, Roger (CDC/CGH/DGHA) (CTR)
<[email protected]<mailto:[email protected]>> wrote:
One reason to keep the empty string rather than null is collating order; null
typically appears last, empty string first.
Re what Ben says about person_attribute, I believe it is conventional
throughout OpenMRS not to make a record to store a null value (attributes, obs,
IDs), so if some required data is missing from an otherwise complete set of
data elements (the patient forgot his national ID card, the vaccine
manufacturer is missing from the mass campaign report, the patient forgot his
ID card from another facility), the empty string is needed as a placeholder
until that information becomes available.
All of which is to agree with Burke that there are sometimes semantic reasons
to distinguish empty string from null.
I would put this in the API in the setters, along with trimming/padding.
-----Original Message-----
From: [email protected]<mailto:[email protected]>
[mailto:[email protected]<mailto:[email protected]>] On Behalf Of Ben Wolfe
Sent: Monday, September 12, 2011 3:19 AM
To:
[email protected]<mailto:[email protected]>
Subject: Re: [OPENMRS-DEV] Empty string fields vs null fields
If it was a runtime property switch that means that users/admins are
the ones making the decision. But it is not them but the developer
that should be deciding how to treat empty strings.
I like the automatic solution, but I fear there might be places where
a column is not nullable but the user doesn't want a value there.
That would end up with us (or module developer) forcing a space into
the value to get around it, and that would definitely make things
worse. (I think person_attribute.value is like that now)
So my vote would be to make it automagic by default but with the
annotation to remove that magic.
And -1000 for switching to Oracle. Oh wait, Oracle already owns MySQL. Crap.
Ben
On Sat, Sep 10, 2011 at 12:06 AM, Saptarshi Purkayastha
<[email protected]<mailto:[email protected]>> wrote:
> +1 for it to be done through the API, without automagic, but probably a
> runtime-property to allow selecting automatic "" or null conversion
> -0 for switching to Oracle
> -1 for the annotation in code
> ---
> Regards,
> Saptarshi PURKAYASTHA
>
> My Tech Blog: http://sunnytalkstech.blogspot.com
> You Live by CHOICE, Not by CHANCE
>
>
> On 10 September 2011 02:00, Burke Mamlin
> <[email protected]<mailto:[email protected]>> wrote:
>>
>> I believe this should be done at the API level, in order to benefit other
>> applications that use the API. I don't think it should be done
>> automagically (e.g., with an OpenmrsObjectSaveHandler) for two reasons: (1)
>> there may be cases were an empty string and null are semantically different
>> and (2) aspect-oriented changes like this are not without a cost (they can
>> add up to real performance hits).
>> How about an annotation on the property like
>> @BlankValue(persist=NULL|BLANK)? . or maybe we should just switch to Oracle.
>> ;-)
>> -Burke
>> On Fri, Sep 9, 2011 at 3:52 PM, Darius Jazayeri
>> <[email protected]<mailto:[email protected]>>
>> wrote:
>>>
>>> Hi All,
>>> The way that our webapp, our API, and the DB interact, we frequently run
>>> into the case where we store empty strings in the database for values, when
>>> they really should be null.
>>> For example if you save a Concept Source and leave the HL7 Code field
>>> blank, you save a row with concept_source.hl7_code = '' rather than null.
>>> (Because the webapp submits an empty input, and we use spring MVC to bind
>>> that to a bean property.)
>>> Two tickets that I looked at today are related to this (and there are
>>> certainly others):
>>> https://tickets.openmrs.org/browse/TRUNK-2516
>>> https://tickets.openmrs.org/browse/TRUNK-2004
>>> We should try to fix this problem in a general way. At first thought I
>>> can't come up with a case where we really want to save the empty string in a
>>> database field rather than null. So some possible solutions are:
>>>
>>> (at the web level) use a custom PropertyEditor for the String class in
>>> all our Spring controllers, so that if you submit "" and bind that to a
>>> String property, it sets it to null.
>>> (at the API level) in OpenmrsObjectSaveHandler iterate over all String
>>> properties and if any of them are "" then set them to null.
>>>
>>> (The first would allow API consumers to explicitly set empty string
>>> properties on objects, while the second wouldn't.)
>>> Any thoughts on this?
>
________________________________
Click here to
unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l>
from OpenMRS Developers' mailing list
________________________________
Click here to
unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l>
from OpenMRS Developers' mailing list
________________________________
Click here to
unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l>
from OpenMRS Developers' mailing list
________________________________
Click here to
unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l>
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]
