0

I am trying to sort a table with columns with both words and numbers. The words are work as intended. But the numbers-part is a bit off.

One of the columns is a price-columns with amounts from 500-1500. But right now, 500 comes below 1500 and I don't know how to fix this.

function sortTable(n) {
  var table, rows, switching, i, x, y, shouldSwitch, dir, switchcount = 0;
  table = document.getElementById("allCars");
  switching = true;

  dir = "asc";

  while (switching) {
      switching = false;
      rows = table.getElementsByTagName("TR");
      for (i = 1; i < (rows.length - 1); i++) {
        shouldSwitch = false;
        x = rows[i].getElementsByTagName("TD")[n];
        y = rows[i + 1].getElementsByTagName("TD")[n];
        if (dir == "asc") {
            if (x.innerHTML.toLowerCase() > y.innerHTML.toLowerCase()) {
            shouldSwitch= true;
            break;
        }
      } else if (dir == "desc") {
        if (x.innerHTML.toLowerCase() < y.innerHTML.toLowerCase()) {
            shouldSwitch= true;
            break;
        }
      }
    }
    if (shouldSwitch) {
      rows[i].parentNode.insertBefore(rows[i + 1], rows[i]);
      switching = true;
      switchcount ++;
    } else {
      if (switchcount == 0 && dir == "asc") {
        dir = "desc";
        switching = true;
      }
    }
  }
}
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
  • The reason your sorts are breaking for integers but are working for strings is because it's sorting the integers as a string so, to compare 1500 to 500 the problem is that 1 is less than 5. This answer explains it in more detail (and better) https://softwareengineering.stackexchange.com/a/127644 Maybe consider an attribute on the top level row (th) of the table which defines it's type, then you can use parseInt to parse a value which you know will be an int – Robbie May 30 '17 at 18:13
  • 1
    rather than do this manually, dataTables (datatables.net) would work better. but if you are averse to that, check if it is a number, then convert the values to a number using Number(val) – Simon May 30 '17 at 18:14

2 Answers2

1

This happens cause you sort method order the numeric values as a string, so 1500 comes before 500 cause 1 come before 5.

You need to cast the numeric values before compare.

you need something like this: (between HERE comments)

function sortTable(n) {


var table, rows, switching, i, x, y, shouldSwitch, dir, switchcount = 0;
  table = document.getElementById("allCars");
  switching = true;

  dir = "asc";

  while (switching) {
      switching = false;
      rows = table.getElementsByTagName("TR");
      for (i = 1; i < (rows.length - 1); i++) {
        shouldSwitch = false;
        x = rows[i].getElementsByTagName("TD")[n];
        y = rows[i + 1].getElementsByTagName("TD")[n];
        if (dir == "asc") {
            if (x.innerHTML.toLowerCase() > y.innerHTML.toLowerCase()) {
            shouldSwitch= true;
            break;
        }
      } else if (dir == "desc") {
        // START HERE
        var value1 = x.innerHTML;
        var value2 = y.innerHTML;

        //CHECK IF VALUES ARE NUMERIC. (LINK LATER)

        if (isNumeric(value1) && isNumeric(value2)){
            if(Number(value1) < Number(value2)){
                shouldSwitch = true;
                break;
            }
        } else {

        // FINISH HERE

            if (x.innerHTML.toLowerCase() < y.innerHTML.toLowerCase()) {
                shouldSwitch = true;
                break;
        }
      }
    }
    if (shouldSwitch) {
      rows[i].parentNode.insertBefore(rows[i + 1], rows[i]);
      switching = true;
      switchcount ++;
    } else {
      if (switchcount == 0 && dir == "asc") {
        dir = "desc";
        switching = true;
      }
    }
  }
}

As you can see, my code does not implement the isNumeric(value) function, you can read about validate decimal numbers in JavaScript to implement it!

War
  • 8,539
  • 4
  • 46
  • 98
Toug
  • 136
  • 4
  • Thank you for taking the time to answer. I didn't get it to work this time, deadline passed :) But I will keep this in mind for the next time. – Andreas Pålsson May 30 '17 at 19:00
0

This Code is Running Fine.

function sortBy(tableId, n){
    var table, rows, switching, i, x, y, shouldSwitch, dir, switchcount = 0;
    table = document.getElementById(tableId);
    switching = true;
    //Set the sorting direction to ascending:
    dir = "asc"; 
    /*Make a loop that will continue until
    no switching has been done:*/
    while (switching) {
        //start by saying: no switching is done:
        switching = false;
        rows = table.getElementsByTagName("TR");
        /*Loop through all table rows (except the
        first, which contains table headers):*/
        for (i = 1; i < (rows.length - 1); i++) {
            //start by saying there should be no switching:
            shouldSwitch = false;
            /*Get the two elements you want to compare,
            one from current row and one from the next:*/
            x = rows[i].getElementsByTagName("TD")[n];
            y = rows[i + 1].getElementsByTagName("TD")[n];
            /*check if the two rows should switch place,
            based on the direction, asc or desc:*/
            if (dir == "asc") {
                if (n == 0){
                    var value1 = x.innerHTML;
                    var value2 = y.innerHTML;
                    if(Number(value1) > Number(value2)){
                        shouldSwitch = true;
                        break;
                    }
                } else {
                    if (x.innerHTML.toLowerCase() > y.innerHTML.toLowerCase()) {
                    //if so, mark as a switch and break the loop:
                    shouldSwitch= true;
                    break;
                }
            }
        }
        else if (dir == "desc") {
            if (n == 0){
                var value1 = x.innerHTML;
                var value2 = y.innerHTML;
                if(Number(value1) < Number(value2)){
                    shouldSwitch = true;
                    break;
                }
            } else {
                if (x.innerHTML.toLowerCase() < y.innerHTML.toLowerCase()) {
                    //if so, mark as a switch and break the loop:
                    shouldSwitch= true;
                    break;
                }
            }
        }
        }
        if (shouldSwitch) {
            /*If a switch has been marked, make the switch
            and mark that a switch has been done:*/
            rows[i].parentNode.insertBefore(rows[i + 1], rows[i]);
            switching = true;
            //Each time a switch is done, increase this count by 1:
            switchcount ++;      
        }
        else {
            /*If no switching has been done AND the direction is "asc",
            set the direction to "desc" and run the while loop again.*/
            if (switchcount == 0 && dir == "asc") {
                dir = "desc";
                switching = true;
            }
        }
    }
}
Saeed Zhiany
  • 2,051
  • 9
  • 30
  • 41
  • please add some nice explanation for your code to show how it solves the problem and avoid posting code only answers. – Saeed Zhiany Nov 13 '19 at 06:01
  • Hi @SaeedZhiany, this code self-explanatory, it doesn't require any more explanation. – RamJi Sharma Nov 13 '19 at 12:39
  • Code only answers will be marked as "not an answer" in most cases by reviewers and it may be removed. – Saeed Zhiany Nov 13 '19 at 12:46
  • you may need to take a look at these links [How do I write a good answer?](https://stackoverflow.com/help/how-to-answer) and [Why and how are some answers deleted?](https://stackoverflow.com/help/deleted-answers) – Saeed Zhiany Nov 13 '19 at 12:50