0

This question is related to, and based off the script from, this question: How does one sort a multi dimensional array by multiple columns in JavaScript?. My solution worked at the time (or so I believe), but after making a series of changes to our custom tag to make it cross-browser compliant, I'm now having a new issue.

So I have the following piece of code. My apologies for the length; this was as far as I could simplify it while still replicating the issue:

<table border=1>
    <tr style="font-weight:bold;">
        <td>Begin Text</td>
        <td>Begin Number</td>
        <td>Sorted Text</td>
        <td>Sorted Number</td>
    </tr>
    <tr>
        <td id="r1c1"></td>
        <td id="r1c2"></td>
        <td id="r1c3"></td>
        <td id="r1c4"></td>
    </tr>
    <tr>
        <td id="r2c1"></td>
        <td id="r2c2"></td>
        <td id="r2c3"></td>
        <td id="r2c4"></td>
    </tr>
    <tr>
        <td id="r3c1"></td>
        <td id="r3c2"></td>
        <td id="r3c3"></td>
        <td id="r3c4"></td>
    </tr>
    <tr>
        <td id="r4c1"></td>
        <td id="r4c2"></td>
        <td id="r4c3"></td>
        <td id="r4c4"></td>
    </tr>
    <tr>
        <td id="r5c1"></td>
        <td id="r5c2"></td>
        <td id="r5c3"></td>
        <td id="r5c4"></td>
    </tr>
    <tr>
        <td id="r6c1"></td>
        <td id="r6c2"></td>
        <td id="r6c3"></td>
        <td id="r6c4"></td>
    </tr>
    <tr>
        <td id="r7c1"></td>
        <td id="r7c2"></td>
        <td id="r7c3"></td>
        <td id="r7c4"></td>
    </tr>
    <tr>
        <td id="r8c1"></td>
        <td id="r8c2"></td>
        <td id="r8c3"></td>
        <td id="r8c4"></td>
    </tr>
    <tr>
        <td><input type="button" onclick="testSort(1);" value="Sort"></td>
        <td><input type="button" onclick="testSort(2);" value="Sort"></td>
    </tr>
</table>

<script>
    function testSort(orderCol) {
        orderList = [orderCol];
        dataArr = do2DArraySort(dataArr, orderList, 'desc');
        for (x=1; x<=numRows; x++) {
            document.getElementById('r' + x + 'c3').innerHTML = dataArr[x-1][1];
            document.getElementById('r' + x + 'c4').innerHTML = dataArr[x-1][2];
        }
    }

    function TwoDimensionalArray(iRows, iCols) { 
        var i;
        var j;
        var a = new Array(iRows);
        for (i=0; i < iRows; i++) {
            a[i] = new Array(iCols);
            for (j=0; j < iCols; j++) {
                a[i][j] = "";
            }
        }
        return(a);
    }

    function do2DArraySort(dataArr, orderList, orderDir) {
        //Loop over each item in the list of sort columns.  For each one invoke the sort method on the array using the appropriate function.
        for (x=orderList.length-1; x >= 0; x--) {
            if (orderDir[x] == 'asc') {
                dataArr.sort(sortMethodFunctionAsc);
            } else {
                dataArr.sort(sortMethodFunctionDesc);
            }
        }

        return dataArr;
    }

    function checkSortValues(a, b) {
        var dataType = 'Text';
        if ((IsNumeric(a) && IsNumeric(b)) || (a == null && IsNumeric(b)) || (IsNumeric(a) && b == null)) {
            dataType = 'Numeric';
        }
        if ((IsDate(a) && IsDate(b)) || (a == null && IsDate(b)) || (IsDate(a) && b == null)) {
            dataType = 'Date';
        }
        return dataType;
    }

    function sortMethodFunctionAsc(a, b) {
        if (checkSortValues(a[orderList[x]], b[orderList[x]]) == 'Numeric') {
            //If the values are numeric, simply check which is larger than the other.
            return a[orderList[x]] - b[orderList[x]];
        } else if (checkSortValues(a[orderList[x]], b[orderList[x]]) == 'Date') {
            //If the values are dates they need converted to date objects.  95% of the time this is not necessary as they are already passed in as dates,
            //but the conversion is required to catch the few cases when they are not.
            var a2 = new Date(a[orderList[x]]);
            var b2 = new Date(b[orderList[x]]);
            //The getTime method is used to convert the dates into millisecond ticker equivalents for easier comparison.
            return a2.getTime() - b2.getTime();
        } else {
            //If one of the values is a string, convert both to a string and compare alphabetically.
            if (a[orderList[x]].toString() > b[orderList[x]].toString()) {
                return 1;
            } else if (a[orderList[x]].toString() < b[orderList[x]].toString()) {
                return -1;
            } else {
                //If they are the same, tell the sort to skip them.
                return 0;
            }
        }
    }

    function sortMethodFunctionDesc(a, b) {
        //This function is identical to the ascending one, but the comparison operators are reversed.
        if (checkSortValues(a[orderList[x]], b[orderList[x]]) == 'Numeric') {
            return b[orderList[x]] - a[orderList[x]];
        } else if (checkSortValues(a[orderList[x]], b[orderList[x]]) == 'Date') {
            var a2 = new Date(a[orderList[x]]);
            var b2 = new Date(b[orderList[x]]);
            return b2.getTime() - a2.getTime();
        } else {
            if (a[orderList[x]].toString() < b[orderList[x]].toString()) {
                return 1;
            } else if (a[orderList[x]].toString() > b[orderList[x]].toString()) {
                return -1;
            } else {
                return 0;
            }
        }
    }

    function IsNumeric(input) {
        return (input - 0) == input && input.length > 0;
    }

    function IsDate(testValue) {
        var returnValue = false;
        var testDate;
        try {
            testDate = new Date(testValue);
            if (!isNaN(testDate)) {
                returnValue = true;
            } else {
                returnValue = false;
            }
        }
        catch (e) {
            returnValue = false;
        }
        return returnValue;
    }

    numRows = 8;
    orderList = [1];
    dataArr = TwoDimensionalArray(numRows, 2);

    dataArr[0][1] = 'Jimbo';
    dataArr[0][2] = 3;
    dataArr[1][1] = 'Jim';
    dataArr[1][2] = 0.65;
    dataArr[2][1] = 'Jackie';
    dataArr[2][2] = 1.25;
    dataArr[3][1] = 'John';
    dataArr[3][2] = 0.8;
    dataArr[4][1] = 'Jacob';
    dataArr[4][2] = 0.95;
    dataArr[5][1] = 'Jill';
    dataArr[5][2] = 0.85;
    dataArr[6][1] = 'Jan';
    dataArr[6][2] = 0.8;
    dataArr[7][1] = 'Jamie';
    dataArr[7][2] = 1.45;
    for (x=1; x<=numRows; x++) {
        document.getElementById('r' + x + 'c1').innerHTML = dataArr[x-1][1];
        document.getElementById('r' + x + 'c2').innerHTML = dataArr[x-1][2];
    }
</script>

If you open this in a browser, you will see an html table that spits out each of the array columns in the order they were entered (ie: random). If you click either of the buttons at the bottom of a column, it will sort the data and place it in the 3rd and 4th columns. My real application wipes and reloads the table, but this works for demonstration purposes.

Clicking the sort button at the bottom of the text column works fine; it sorts the array by names alphabetically. However, if you sort the number column you will see a change in order but by no means correct. The results are the same each time, but different if you click the number sort immediately or after a text search (since the search is based off the current state of the array and not the original array). This is exactly what I'm seeing in my application.

It took me a while to narrow it down, but here's what I've determined:

1) Text, date, and integers sort fine; only decimals do not.

2) Decimals sort according to the first digit, but ignore data after the decimal point.

3) Multi-column sort works fine, but shows this same anomaly, so I simplified it to a single-column sort for this example.

I've been staring at this for hours, but without luck, and could use a fresh set of eyes. Why is my array.sort function not working correctly? Any thoughts would be appreciated.

Community
  • 1
  • 1
Nicholas
  • 1,974
  • 4
  • 20
  • 46
  • I'd really consider not duplicating all the logic in the asc/desc methods; rather have a single function that you can call with its parameters swapped. Easy to make a trivial mistake, not as easy to see it. – Dave Newton Dec 08 '11 at 19:29
  • Also a nerve-wracking use of `x`; why not just pass it and keep things safe? In any case, are you sure the data is being identified the way you think it is, and that you're operating on the data you think you are? The addition/subtraction will work for decimals if they're being interpreted as decimals (even if they're strings, even if one is `null`). – Dave Newton Dec 08 '11 at 19:34
  • @Dave, Thanks for the thoughts. I originally wrote a single sort function, but it looked much more confusing which is why I broke it into two. Maybe I just approached it wrong that first time. Regarding x, where is it that you feel I'd be better off replacing it with a simplified identifier? – Nicholas Dec 08 '11 at 19:49
  • 1
    Simplified? No, I'm suggesting you make it local, and pass it to the function, instead of relying in a global between the two functions. – Dave Newton Dec 08 '11 at 19:56
  • May I suggest `function sortMethodFunctionDesc(a, b) { return - sortMethodFunctionAsc(a, b); }` instead of repeating all the code in the function? – ErikE Mar 28 '13 at 17:52

2 Answers2

1

There is a problem in your checkSortValues function where once the data type is set to number, it then tests positive for date also.

Also, you have an error in your IsNumeric test, where the input.length will return undefined. You should comment out/change the second part of the test.

function IsNumeric(input) {
  return (input - 0) == input // && input.length > 0;
}

If you change the IsNumeric test and if you use else if instead of if for the date test, I think it solves your issue.

function checkSortValues(a, b) {
    var dataType = 'Text';
    if ((IsNumeric(a) && IsNumeric(b)) || (a == null && IsNumeric(b)) || (IsNumeric(a) && b == null)) {
        dataType = 'Numeric';
    }
    else if ((IsDate(a) && IsDate(b)) || (a == null && IsDate(b)) || (IsDate(a) && b == null)) {
        dataType = 'Date';
    }
    return dataType;
}

Edit: Include full code with my suggested modifications...

I have tested this, and it works for me, sorting in the correct order.

<table border=1>
    <tr style="font-weight:bold;">
        <td>Begin Text</td>
        <td>Begin Number</td>
        <td>Sorted Text</td>
        <td>Sorted Number</td>
    </tr>
    <tr>
        <td id="r1c1"></td>
        <td id="r1c2"></td>
        <td id="r1c3"></td>
        <td id="r1c4"></td>
    </tr>
    <tr>
        <td id="r2c1"></td>
        <td id="r2c2"></td>
        <td id="r2c3"></td>
        <td id="r2c4"></td>
    </tr>
    <tr>
        <td id="r3c1"></td>
        <td id="r3c2"></td>
        <td id="r3c3"></td>
        <td id="r3c4"></td>
    </tr>
    <tr>
        <td id="r4c1"></td>
        <td id="r4c2"></td>
        <td id="r4c3"></td>
        <td id="r4c4"></td>
    </tr>
    <tr>
        <td id="r5c1"></td>
        <td id="r5c2"></td>
        <td id="r5c3"></td>
        <td id="r5c4"></td>
    </tr>
    <tr>
        <td id="r6c1"></td>
        <td id="r6c2"></td>
        <td id="r6c3"></td>
        <td id="r6c4"></td>
    </tr>
    <tr>
        <td id="r7c1"></td>
        <td id="r7c2"></td>
        <td id="r7c3"></td>
        <td id="r7c4"></td>
    </tr>
    <tr>
        <td id="r8c1"></td>
        <td id="r8c2"></td>
        <td id="r8c3"></td>
        <td id="r8c4"></td>
    </tr>
    <tr>
        <td><input type="button" onclick="testSort(1);" value="Sort"></td>
        <td><input type="button" onclick="testSort(2);" value="Sort"></td>
    </tr>
</table>

<script>
    function testSort(orderCol) {
        orderList = [orderCol];
        dataArr = do2DArraySort(dataArr, orderList, 'desc');
        for (x=1; x<=numRows; x++) {
            document.getElementById('r' + x + 'c3').innerHTML = dataArr[x-1][1];
            document.getElementById('r' + x + 'c4').innerHTML = dataArr[x-1][2];
        }
    }

    function TwoDimensionalArray(iRows, iCols) { 
        var i;
        var j;
        var a = new Array(iRows);
        for (i=0; i < iRows; i++) {
            a[i] = new Array(iCols);
            for (j=0; j < iCols; j++) {
                a[i][j] = "";
            }
        }
        return(a);
    }

    function do2DArraySort(dataArr, orderList, orderDir) {
        //Loop over each item in the list of sort columns.  For each one invoke the sort method on the array using the appropriate function.
        for (x=orderList.length-1; x >= 0; x--) {
            if (orderDir[x] == 'asc') {
                dataArr.sort(sortMethodFunctionAsc);
            } else {
                dataArr.sort(sortMethodFunctionDesc);
            }
        }

        return dataArr;
    }

    function checkSortValues(a, b) {
        var dataType = 'Text';
        if ((IsNumeric(a) && IsNumeric(b)) || (a == null && IsNumeric(b)) || (IsNumeric(a) && b == null)) {
            dataType = 'Numeric';
        }
        else if ((IsDate(a) && IsDate(b)) || (a == null && IsDate(b)) || (IsDate(a) && b == null)) {
            dataType = 'Date';
        }
        return dataType;
    }

    function sortMethodFunctionAsc(a, b) {
        if (checkSortValues(a[orderList[x]], b[orderList[x]]) == 'Numeric') {
            //If the values are numeric, simply check which is larger than the other.
            return a[orderList[x]] - b[orderList[x]];
        } else if (checkSortValues(a[orderList[x]], b[orderList[x]]) == 'Date') {
            //If the values are dates they need converted to date objects.  95% of the time this is not necessary as they are already passed in as dates,
            //but the conversion is required to catch the few cases when they are not.
            var a2 = new Date(a[orderList[x]]);
            var b2 = new Date(b[orderList[x]]);
            //The getTime method is used to convert the dates into millisecond ticker equivalents for easier comparison.
            return a2.getTime() - b2.getTime();
        } else {
            //If one of the values is a string, convert both to a string and compare alphabetically.
            if (a[orderList[x]].toString() > b[orderList[x]].toString()) {
                return 1;
            } else if (a[orderList[x]].toString() < b[orderList[x]].toString()) {
                return -1;
            } else {
                //If they are the same, tell the sort to skip them.
                return 0;
            }
        }
    }

    function sortMethodFunctionDesc(a, b) {
        //This function is identical to the ascending one, but the comparison operators are reversed.
        if (checkSortValues(a[orderList[x]], b[orderList[x]]) == 'Numeric') {
            return b[orderList[x]] - a[orderList[x]];
        } else if (checkSortValues(a[orderList[x]], b[orderList[x]]) == 'Date') {
            var a2 = new Date(a[orderList[x]]);
            var b2 = new Date(b[orderList[x]]);
            return b2.getTime() - a2.getTime();
        } else {
            if (a[orderList[x]].toString() < b[orderList[x]].toString()) {
                return 1;
            } else if (a[orderList[x]].toString() > b[orderList[x]].toString()) {
                return -1;
            } else {
                return 0;
            }
        }
    }

    function IsNumeric(input) {
      return (input - 0) == input// && input.length > 0;
    }

    function IsDate(testValue) {
        var returnValue = false;
        var testDate;
        try {
            testDate = new Date(testValue);
            if (!isNaN(testDate)) {
                returnValue = true;
            } else {
                returnValue = false;
            }
        }
        catch (e) {
            returnValue = false;
        }
        return returnValue;
    }

    numRows = 8;
    orderList = [1];
    dataArr = TwoDimensionalArray(numRows, 2);

    dataArr[0][1] = 'Jimbo';
    dataArr[0][2] = 3;
    dataArr[1][1] = 'Jim';
    dataArr[1][2] = 0.65;
    dataArr[2][1] = 'Jackie';
    dataArr[2][2] = 1.25;
    dataArr[3][1] = 'John';
    dataArr[3][2] = 0.8;
    dataArr[4][1] = 'Jacob';
    dataArr[4][2] = 0.95;
    dataArr[5][1] = 'Jill';
    dataArr[5][2] = 0.85;
    dataArr[6][1] = 'Jan';
    dataArr[6][2] = 0.8;
    dataArr[7][1] = 'Jamie';
    dataArr[7][2] = 1.45;
    for (x=1; x<=numRows; x++) {
        document.getElementById('r' + x + 'c1').innerHTML = dataArr[x-1][1];
        document.getElementById('r' + x + 'c2').innerHTML = dataArr[x-1][2];
    }
</script>

You can confirm that here if you want to...

Billy Moon
  • 57,113
  • 24
  • 136
  • 237
  • Thank you for the suggestion, but this did not resolve the problem. Throwing an alert into the mix it looks like the dataType is being properly identified as a number for sorting purposes. – Nicholas Dec 08 '11 at 19:46
  • @NicholasBostaph You sure? `IsDate("5.5")` will return true; w/o the `else` the `dataType` variable will be set to `Date` if both numbers are decimals. Since you don't return as soon as you have a type, I'm not convinced you're correct. – Dave Newton Dec 08 '11 at 19:53
  • Billy, thank you; that worked perfectly! It's possible that I was incorrect in my earlier comment. I thought I tested it correctly, but I have 3 versions of this code open right now and it's conceivable that I made a mistake. If so, I apologize. Thanks for your help. :) – Nicholas Dec 08 '11 at 20:35
  • No problem, happy to help. This was a tricky problem, because there were two errors. Fixing one alone did not solve the problem. Good luck and godspeed! – Billy Moon Dec 08 '11 at 20:46
1

I don't know about Dates but Strings and Numbers can both be sorted with:

if (a < b) {
    return 1;
} else if (b > a) {
    return -1;
} else {
    return
}

The < and > work for both datatypes. I changed this in your code and it works fine: http://jsfiddle.net/Ft44x/6/ I've changed the sortDesc function. Any other bugs are probably caused by your code being too convoluted, clean it up!

Also, sorting the other way around is just: no need to implement the same thing twice.

function sortAsc() {
    ... // implement
}

function sortDesc(a, b) {
    return -1 * sortAsc(a,b);
}

Also, if you have a number don't store it as a string, that's just silly.

var my_string = "foo";
var my_number = 42; // no quotes
var my_decimal = 0.63;

One more thing, if you need to know if something is a Date or not, why not use instanceof?

var date = new Date();
alert(date instanceof Date);    // should be true

You can do the same thing for Strings and Numbers but you don't even need to :P

Halcyon
  • 57,230
  • 10
  • 89
  • 128
  • I linked to the wrong jsFiddle earlier, in case you caught it, this one works: http://jsfiddle.net/Ft44x/6/ – Halcyon Dec 08 '11 at 20:03
  • +1 These are excellent suggestions and I will begin integrating them today; I'm a big proponent of clean code, but am a JS intermediate and much of this stuff was all new to me when I wrote it. Thanks for your help. – Nicholas Dec 08 '11 at 20:40
  • Ah, I remember now why I didn't use your first suggestion and check the data type first. Sometimes data columns will contain both strings and numbers, and I'm getting an error when trying to compare them. As this is a custom tag, I can't control the input (which comes from the DB and goes through several other scripts); I just need to work with it. But "return -1 * sortAsc(a,b)" had me kicking myself and instanceof is something I never knew about before; thanks. :) – Nicholas Dec 08 '11 at 20:49