[psa] JSONObject.optString doesn't do what you think it does

James Hugman jhugman at mozilla.com
Fri May 23 09:00:38 PDT 2014


Hi, mfd

As another datapoint: the Obj-C category `NSObject+NSJSON` treats nulls 
as a separate object type: `NSNull`, and all instances are 
`NSNull.null`. Instances of JS's undefined is represented with the 
equivalent of Java's `null`: `nil`. This is only possible because of 
Objective-C's loose typing, and will not transliterate into Java.

Amongst my unsolicited opinions are these:

  * code which changes behaviours based on a difference between 
`undefined` and `null` should be avoided. Better yet, it should be 
shunned or sent for re-education.
  * checking if a key exists if different to checking if the value is 
`null`. JSONObject has a method `has` which does this. Javans almost 
never use the `Map.containsKey`, but if we really need to distinguish 
between JS `null` and `undefined`, then we should be using this more.

The usual disclaimers here: purity v pragmatism, refactoring/rewriting 
takes time, the wrong-end-of-the-stick etc

- James



On 5/23/14, 1:28 AM, Nick Alexander wrote:
> Hi m-f-d,
>
> Just a friendly reminder that JSONObject.optString doesn't do what you 
> think it does.  For terrible reasons documented at [1], if o = 
> {'key':null}, then
>
> JSONObject(o).optString("key") == "null"
>
> and
>
> JSONObject(o).optString("key", null) == "null"
>
> That's right.  I discovered a few places in Fennec where 
> NativeJSObject does a different thing [2], and then started to look at 
> our use of optString, and it's not pretty.  Way too many uses to audit 
> :(  So instead of trying to fix, I'm trying to educate instead.  Be 
> careful when passing null from JS to Java!
>
> We could try to make NativeJSObject do the right thing, but we have 
> already gone quite far down the path of NativeJSObject is a drop-in 
> replacement for JSONObject; I don't think we can go back. And 
> converting between the two conventions is as hard as auditing all 
> current uses, amortized over time.
>
> Nick
>
> [1] http://stackoverflow.com/a/23377941
>
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1014965
> _______________________________________________
> mobile-firefox-dev mailing list
> mobile-firefox-dev at mozilla.org
> https://mail.mozilla.org/listinfo/mobile-firefox-dev



More information about the mobile-firefox-dev mailing list