Patch for dot notation handling null objects

[hlascelles]

Imagine you have a column define thus:

"mDataProp": "person.firstname", "sTitle": "First Name"
"mDataProp": "alternativeperson.firstname", "sTitle": "First Name"

The "person" is always present, so it succeeds. But the "alternativeperson" may be null. When that happens, DataTables fails.

This patch checks the value chain for nulls when using dot notation.

Great project, thanks!

H

[DataTables]

Thanks for the patch - that looks like it could be a nice little enhancement. There is a discussion in the DataTables forums at the moment about what to do when properties don't exist - in this case you have the parent property, but it is null, so it more or less falls into this same question. The proposal is to use a try/catch when working with mDataProp - if the property fails to be read then the catch is to use the default content. The question is, how does this effect performance, if at all (and also usability for when you don't actually intend to do this!).

[hlascelles]

Good points. My tables are small so performance is not an issue.
My rule of thumb (thankyou Joshua Bloch) is never to use try/catch for
control flow. But I don't think performance is included in that rule.

Will keep an eye on how the conversation proceeds, but my vote is (excuse
the pseudo code, I'm primarily a Java dev)

1. If the main property is null, call an overriddable valueForNull() method
2. If there is a null pointer in a dot annotation traverse, see 1.

Either way, terrific product. Will donate!

H

On 21 November 2011 09:02, Allan Jardine <
reply@reply.github.com

wrote:

Thanks for the patch - that looks like it could be a nice little
enhancement. There is a discussion in the DataTables forums at the moment
about what to do when properties don't exist - in this case you have the
parent property, but it is null, so it more or less falls into this same
question. The proposal is to use a try/catch when working with mDataProp -
if the property fails to be read then the catch is to use the default
content. The question is, how does this effect performance, if at all (and
also usability for when you don't actually intend to do this!).

---

Reply to this email directly or view it on GitHub:
#39 (comment)

[DataTables]

This has now been addressed with commit 468390c . Sorry I forgot to update this pull request.

Now when mDataProp was used to get a nested object, but a parent object didn't exist it would throw an unrecoverable error. With this change the behaviour matches that of single level data whereby if data cannot be found, at any level, then undefined is returned from the data get object. This means that if sDefaultContent is defined then that will be used instead, and if not defined an error will still be given (although this one under DataTables' control).

[pzgz]

I am afraid this issue is still there.

For eg., if the data object is like:

{
  a: "xxx",
  b: null
}

In this example, the node "b" should be an object, but it's actually NULL right now. If so, I will get an error from datatable js. Because it's trying to get data by data[ a[i] ], which data is NULL.

If the node "b" is not presented, everything is fine.

I have made simple patch on line 822 and line 876, to check data before retrieve properties from them. But not sure if it's ok for submitting a pull request on this.

line 822:

if (data != null) data = data[ a[i] ];

line 876:

if (data != null) data[ a[a.length-1] ] = val;

[DataTables]

I'm been thinking about this for the last hour or so, playing around with various options. The reason I'm hesitant to just fix it is that null !== undefined, which is what the original bug covered - so null here is actually a different, although related issue.

However, I think that it probably is correct to treat null like undefined here, so I've committed the above change, along with the unit test, to allow null values like that. I do reserve the right to change it again in future for null values though, should this prove to be problematic :-)

[pzgz]

Fair enough, thanks a lot, and this is a really good plugin. Thanks.