So, does that mean you're fine with automagically changing empty String properties of OpenmrsObjects to null before saving to the database?
-Darius On Thu, Sep 22, 2011 at 11:43 AM, Burke Mamlin <[email protected]>wrote: > On Thu, Sep 22, 2011 at 9:18 AM, Darius Jazayeri > <[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]> 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]] *On Behalf Of *Ben >>> Wolfe >>> *Sent:* Thursday, September 22, 2011 7:54 AM >>> >>> *To:* [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]> 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]> 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]] On Behalf Of Ben Wolfe >>> Sent: Monday, September 12, 2011 3:19 AM >>> To: [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]> 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]> >>> 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]> >>> >> 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<[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]
