Problem with order() function

awelch

There seems to be a problem with the order() function. Since there is an API method to set the column order I would expect this method to return a copy of the datatable order settings when it is called without parameters however, it is returning a direct reference to datatables internal settings. This means that changing the returned array will change the ordering on the table. You can see this behavior demonstrated in this JS Bin: http://live.datatables.net/zikutuhu/1/edit. The documentation isn't explicit as to whether this is the intended behavior of the function but it seems like bad practice to expose the internal settings in this way. I have run into a problem with this when trying to get the sorting order while using colReorder. The array from .order() must be adjusted based on the column order from colReorder.order() in order to be used to initialize the table in the future. However, changing the returned array will change the live table so the returned array must be cloned before altering the data. This isn't show stopping but it should be a pretty easy fix.
The problem code being:

_api_register( 'order()', function ( order, dir ) {
...
if ( order === undefined ) {
    // get
    return ctx.length !== 0 ?
        ctx[0].aaSorting :
        undefined;
}
...

This returns a mutable reference to aaSorting. Instead a deep copy could be returned as such:

_api_register( 'order()', function ( order, dir ) {
...
if ( order === undefined ) {
    // get
    return ctx.length !== 0 ?
        $.extend(true, [], ctx[0].aaSorting) :
        undefined;
}
...

As an aside, the order() function seems to have undocumented functionality. Namely, if you pass a column number and a direction to the function (e.g. table.order(1, "asc")) rather than an array of objects it will set sorting on a single column. Not a bug but the documentation may need updating.

allan

I've never been certain if the current behaviour is good practice or not. To a large extent I didn't expect anyone to mutate it externally, so I never really figured it as much of a problem (until now ).

Workaround would be table.order().slice().

Allan

awelch

Hardcore defensive coders would have a cow I'm sure but I suppose it ultimately depends on the use case. In this particular case I would tend to agree with you that it doesn't really matter. I will submit a comment to the order() page with a tip for anyone that needs to modify the returned array and this thread can be closed.

I should mention though, slice() returns a shallow copy so it would essentially be the same as returning the original array. I notice you're using slice in the setter:

return this.iterator( 'table', function ( settings ) {
    settings.aaSorting = order.slice();
} );

This may not be doing what you want but again, probably isn't going to have a noticeable impact on usage. It may just be unnecessary.

Deep copying an array in javascript is a pain. The only ways I've found are with a loop or recursion, JSON.parse(JSON.stringify(arr)) (this method has limitations), I think .map() can be used, and my personal preference jQuery.extend(true, [], arr). The last method is actually an undocumented behavior of the jQuery.extend() function. It's like deep copying an object but you pass an empty array instead of an empty object.

allan

Doh - I forgot about the nested array. Thanks for pointing that out and also your comment on the order() page which is excellent!

Allan