sDefaultContent not used in array of objects if intermediate property does not exist

sDefaultContent not used in array of objects if intermediate property does not exist

ABCABC Posts: 4Questions: 0Answers: 0
edited October 2011 in Bug reports
I am passing an array of objects to a datatable and would like to define a column as, for example, "attr1.attr2.attr3"
Further, not all objects are expected to have a value for attr1.attr2.

So I also provide an sDefaultContent parameter.

However the function _fnGetObjectDataFn expects the entire path attr1.attr2.attr3 to be traversable. If attr1.attr2 is "undefined" then an exception will be thrown.

Instead could I suggest that _fnGetObjectDataFn should just return undefined?

A possible patch is

[code]
--- jquery.dataTables.js 2011-10-05 15:04:54.000000000 +1300
+++ local-jquery.dataTables.js 2011-10-05 15:01:38.000000000 +1300
@@ -6743,23 +6743,29 @@
if ( a.length == 2 )
{
return function (data) {
- return data[ a[0] ][ a[1] ];
+ try {return data[ a[0] ][ a[1] ];}
+ catch(e) {return undefined;}
};
}
else if ( a.length == 3 )
{
return function (data) {
- return data[ a[0] ][ a[1] ][ a[2] ];
+ try {return data[ a[0] ][ a[1] ][ a[2] ];}
+ catch(e) {return undefined;}
};
}
else
{
return function (data) {
+ try
+ {
for ( var i=0, iLen=a.length ; i

Replies

  • allanallan Posts: 61,744Questions: 1Answers: 10,111 Site admin
    edited October 2011
    That sounds like a good plan. I've bookmarked your suggestion to look at adding it into the next version of DataTables.

    Thanks for the suggestion!

    Allan
  • PartTimerPartTimer Posts: 4Questions: 0Answers: 0
    Hi,

    Allan: Firstly, thank you very much for all the hard work you've obviously put in to this project. I rarely sign up for forums, but I had to do so to pass on my thanks.

    ABC: Thanks for the great post - thankfully I spotted your comments describing the exact problem I've just myself run into, so I may I cast my vote for this in the next version of DataTables too.

    Thanks again Allan and the community for all the hard work.
  • allanallan Posts: 61,744Questions: 1Answers: 10,111 Site admin
    Hi PartTimer,

    Fantastic to hear you like DataTables - it really means a lot to get your kind of post, so thanks!

    This is certainly something that want to look at putting into the next revision (which I've just started committing a few things for). My only concern is how much overhead the try/catch block will add, particularly when dealing with large tables, since these return functions need to be as fast as possible, or risk slowing the whole table significantly, That's the first line of research for me to take!

    Regards,
    Allan
  • PartTimerPartTimer Posts: 4Questions: 0Answers: 0
    Here's hoping there is an easy alternative / not too big of a hit in performance.

    In the meantime I'm going to try using the above suggestion. If I have any thoughts regarding its success I'll let you know.
  • PartTimerPartTimer Posts: 4Questions: 0Answers: 0
    And as a quick update ... so far so good. Doesn't seem to have caused a noticeable degradation in performance on the data I'm using at least.

    Thanks again folks.
  • allanallan Posts: 61,744Questions: 1Answers: 10,111 Site admin
    Thanks for letting us know! I would imagine that most modern Javascript engines will cope with this just fine - but there is an extra set in there, so it would be nice to be able to quantify it before sticking it into DataTables at a point which could do a fair amount of harm if it were wrong.

    How many rows / columns are you using in your table and are you using client or server-side processing?

    Thanks,
    Allan
  • PartTimerPartTimer Posts: 4Questions: 0Answers: 0
    To be fair, I'm using server side processing, so the number of rows returned isn't particularly high. The records themselves are coming from a Doctrine based system, each json held record having a number of objects, so I will eventually be drilling down into this data in a click-to-expand-row setup. There is the potential for a large number of objects in a row. I'll possibly see about pulling in more rows than I need to stress test the browser a little.

    Figures wise I would say around 10 columns, default 10 rows per request but regularly test with 100. Each row uses info from two types of objects minimum, and an optional third type could be held in an array with up to 10-20 items. Effectively each row is a conversation , two columns pull info from one object containing info about the creator, one with info about the project, and the final object type is a list of emails, so could have a variable number of emails and responses. These emails are to be displayed in the drill down manner.

    When this is in more of a finished state I can try and provide some more accurate details, if it would help.
  • allanallan Posts: 61,744Questions: 1Answers: 10,111 Site admin
    Brilliant - thanks for the information. Really interesting to hear how you are using DataTables! With server-side processing I can't really see this change to using try/catch making any real difference at all. I'm not concerned about it with a hundred rows, its more when dealing with 50'000+ rows client-side that I would be interested to see if there is a difference. Each try/catch isn't going to cost much, but its whether they all add up to be sizeable or not. I'll see if I can do some testing at some point and post back some figures.

    Regards,
    Allan
  • ABCABC Posts: 4Questions: 0Answers: 0
    edited February 2012
    Hi Allan

    I wonder if you would consider the following slightly different patch for incorporation into 1.9? It avoids the weight of exception handling by checking explicitly for undefined as it traverses the attribute path.

    (Apologies for the whitespace mismatch ... I'm not using tabs)

    [code]
    if ( a.length == 2 )
    {
    return function (data) {
    - return data[ a[0] ][ a[1] ];
    + // traverse the first attribute and check it was defined
    + var data = data[ a[0] ];
    + if (typeof data == 'undefined') {
    + return data;
    + }
    + return data[ a[1] ];
    };
    }
    else if ( a.length == 3 )
    {
    return function (data) {
    - return data[ a[0] ][ a[1] ][ a[2] ];
    + // traverse the first attribute and check it was defined
    + data = data[ a[0] ];
    + if (typeof data == 'undefined') {
    + return data;
    + }
    +
    + // traverse the second attribute and check it was defined
    + data = data[ a[1] ]
    + if (typeof data == 'undefined') {
    + return data;
    + }
    + return data[ a[2] ];
    };
    }
    else
    @@ -6758,6 +6774,9 @@
    for ( var i=0, iLen=a.length ; i
  • allanallan Posts: 61,744Questions: 1Answers: 10,111 Site admin
    Thats a very interesting approach. I wonder if it might be more appropriate to return undefined rather than the original data object, which would allow the default content to "show through".

    I was actually just about to package 1.9.0, but this does seem like a good plan to add in. Let me think about it a little and I'll get back to you :-)

    Allan
  • allanallan Posts: 61,744Questions: 1Answers: 10,111 Site admin
    Been thinking about this and I think its an excellent modification - thanks very much for suggesting it! I've done it slightly differently, returning undefined, which allows the behaviour to match that of a "shallow" parameter that is missing - as such I've actually marked this as a fix rather than a new feature or update - I think the difference between the two was a bug (basically sDefaultContent would be used when a shallow property was missing, but not if a whole parent object was missing).

    I've also removed the "fast lookup" functions as they don't really provide any sizeable speed benefit and they would require a little bit more code overhead.

    Unit tests have also been added to make sure this stays working for all types :-)

    Commit:
    https://github.com/DataTables/DataTables/commit/468390c337caa62f387bc15c5bc54c0313c99093

    This will be in 1.9.0 which will be released soon :-)

    Regards,
    Allan
  • ABCABC Posts: 4Questions: 0Answers: 0
    Thanks thats great Allan ... just as a note ... my suggested patch was also returning undefined for missing attributes.

    It just wasn't particularly explicit ... its doing

    [code]
    if data is undefined:
    return data # but we know that data is the same as undefined!
    [/code]
  • ABCABC Posts: 4Questions: 0Answers: 0
    Also ... in case it is of use ... I currently use similar code for setting attributes on objects where intervening ones may be missing.

    I currently do this better "setting" (and "getting") by providing custom mDataProp functions rather than patching the datatables library ... but perhaps this might also be a candidate for patching in the library?

    It would be in function _fnSetObjectDataFn( mSource ) and look something like:

    [code]
    else if ( typeof mSource === 'string' && mSource.indexOf('.') != -1 )
    {
    /* If there is a . in the source string then the data source is in a nested object
    * we provide two 'quick' functions for the look up to speed up the most common
    * operation, and a generalised one for when it is needed
    */
    var a = mSource.split('.');
    if ( a.length == 2 )
    ...
    else if ( a.length == 3 )
    ...
    else
    {
    return function (data, val) {
    for ( var i=0, iLen=a.length-1 ; i
  • allanallan Posts: 61,744Questions: 1Answers: 10,111 Site admin
    Hi,

    Oops - I missed the return data is undefined - sorry about that! At least we are on the same track!

    Nice suggestion about creating child objects that don't yet exist. As before let me think about it a little bit, but it does seem like a good idea :-)

    Regards,
    Allan
  • DaReaper5DaReaper5 Posts: 4Questions: 0Answers: 0
    Hi, I am having an issue with this code. (Should I start a new thread?)

    An error is thrown ("data is null") for the following when... data is null.

    [code]data = data[ a[i] ];

    if ( data === undefined )
    {
    return undefined;
    } [/code]
    (Line 822 of jquery.datatables.js)

    An example case would be if mDataProp is referencing a nested field that does not exist because the parent object is null.

    How would I alleviate this? (sDefaultContent does not work)
  • allanallan Posts: 61,744Questions: 1Answers: 10,111 Site admin
    @DaReaper5 - Can you link us to a test page which shows the issue please?

    Allan
  • DaReaper5DaReaper5 Posts: 4Questions: 0Answers: 0
    edited October 2012
    Hi I have fixed the issue. I was using 1.9.2 and not 1.9.4.

    The following is a test page that has the error and is using 1.9.2:
    http://live.datatables.net/etacuf/edit#source

    Thanks again for responding allen, apologies for not initially posting a test page (takes a while to recreate the issue :P).
This discussion has been closed.