Sort javascript array by property or function value

Question

I have an array that contains raw values as well as computed values. I would like to be able to sort the array dynamically based on either a raw value or the result of one of the computed values. The actual sorts that will be required will not be known until runtime.

I've put together the below sample (plunker here) that demonstrates the situation and a working solution*. I would like to know how to improve this... specifically, the use of:

Array.prototype.sortBy = function (property) {
    return this.sort(mySort(property));
};

is copied from this stackoverflow response - and Ege Özcan specifically states

//Please don't just copy-paste this code. 
//See the explanation at the end. A lot could break.

I would like to understand how to implement this sorting algorithm on my object without violating the 'A lot could break' warning (that I don't understand).

*One of the things I love about stackoverflow is that the process of framing the question well frequently leads you to simplify the problem to the point where a (not necessarily the) solution presents itself. I started this problem not being able to sort based on a property or computed value. Now, I'm looking for validation/improvement on the implementation.

Sample:

var rawData = [
  { "Id": 3, "itemCount": 3531, "val1": 905, "val2": 172 },
  { "Id": 2, "itemCount": 3111, "val1": 799, "val2": 147 },
  { "Id": 4, "itemCount": 3411, "val1": 871, "val2": 199 },
  { "Id": 5, "itemCount": 3414, "val1": 892, "val2": 178 },
  { "Id": 1, "itemCount": 3182, "val1": 845, "val2": 155 }
];



function MyItem(item) {
    var self = this;
    for (var val in item) {
      if (item.hasOwnProperty(val)) {
          self[val] = item[val];
      }
    }
 }

function extendMyItems() {
    MyItem.prototype.computedOne = function () {
        var input = this;
        return input.itemCount / input.val1;
    };

    MyItem.prototype.computedTwo = function () {
        var input = this;
        return input.val1 * input.val2;
    };
}

function getItems(input) {
    var ret = [];
    for (var i = 0; i < input.length; i++) {
        var item = new MyItem(input[i]);
        ret.push(item);
    }

    return ret;
}

function doIt() {

    Array.prototype.sortBy = function (property) {
        return this.sort(mySort(property));
    };

    extendMyItems();
    var sortList = [{ "sortKey": "Id", "sortOrder": "asc" },
                { "sortKey": "val1", "sortOrder": "asc" },
                { "sortKey": "val2", "sortOrder": "desc" },
                { "sortKey": "computedOne", "sortOrder": "desc", "isComputed": true },
                { "sortKey": "Id", "sortOrder": "desc" },
                { "sortKey": "computedTwo", "sortOrder": "asc", "isComputed": true }];

    // get the array of MyItem
    var myItems = getItems(rawData);

    for (var k = 0; k < sortList.length; k++) {
        myItems.sortBy(sortList[k]);
        // process the sorted items here (ranking/scoring, etc)
        for (var p = 0; p < myItems.length; p++) {
            console.log('Id: ' + myItems[p].Id + ' val1: ' + myItems[p].val1 + ' val2:                                ' + myItems[p].val2 + ' c1: ' + myItems[p].computedOne() + ' c2: ' +  myItems[p].computedTwo());
    }

}

function mySort(srt) {
    var so = srt.sortOrder == 'asc' ? 1 : -1;
    var key = srt.sortKey;
    var result = 0;
    console.log(srt.sortKey + ' ' + srt.sortOrder + ':');

    return function (a, b) {
        if (srt.isComputed) {
            // this seems like a hack - is there a better way to switch between property and function value????
            result = (a[key]() < b[key]()) ? -1 : (a[key]() > b[key]()) ? 1 : 0;
        } else {
            result = (a[key] < b[key]) ? -1 : (a[key] > b[key]) ? 1 : 0;
        }

        return result * so;
    };

    }
}

Answer 1

It is considered by most a bad practice to extend native objects like Array. That is what the post you mentioned was getting at. The concern is you do not know how this will effect the behavior of other scripts.

Here is an example of a scenario where poorly written caused issues once the array prototype was manipulated. This is a scenario I ran into once on a large code based which was really hard to track down:

function BadUseOfForLoop(){
   //You should NEVER use a for in loop to iterate over an array 
   //although some people do this and it works until you extend Array
   var arr = [1,2,3,4];
    for (key in arr){
      console.log(arr[key]);
    }  
}

BadUseOfForLoop();

console.log("Extend Array...");

Array.prototype.sortBy = function(){
   return "Doesnt matter...";
};

BadUseOfForLoop();

Output:

1 
2 
3
4 
Extend Array... 
1 
2 
3 
4 
function (){
   return "Doesnt matter...";
} 

http://jsfiddle.net/vTwRY/

One thing you could do to avoid the caveat is simply not extend the Array object and create a helper to do this for you.

var ArrayHelper = {
    sortBy : function(arr, prop){
        return function(){
            var so = srt.sortOrder == 'asc' ? 1 : -1;
            var key = srt.sortKey;
            var result = 0;
            console.log(srt.sortKey + ' ' + srt.sortOrder + ':');
            return function (a, b) {
                if (srt.isComputed) {
                result = (a[key]() < b[key]()) ? -1 : (a[key]() > b[key]()) ? 1 : 0;
                } else {
                    result = (a[key] < b[key]) ? -1 : (a[key] > b[key]) ? 1 : 0;
                }
                return result * so;
            };
        }
    }
};

And then in your code...

ArrayHelper.sortBy(myItems,sortList[k]);

instead of

myItems.sortBy(sortList[k]);

Working demo

For further reading on the subject this Perfection Kills post discusses whether or not extending native objects is a good idea and you will see it is not a cut and dry issue. Above, I layed out an issue that is not discussed in this blog post but really can cause conflicts with other code.

Answer 2

The answer states that you shouldn't extend things like Array.prototype. Personally, I don't consider this a mortal sin; most libraries can safely handle these scenarios.

That said, you could wrap the sorting functionality:

function ArraySorter(arr)
{
    this.arr = arr;
}

ArraySorter.prototype.sortBy = function(prop) {
    return this.arr.sort(mySort(prop));
}

var sortedItems = new ArraySorter(myItems).sortBy('whatever');