2
\$\begingroup\$

This works fine but I find it quite big and had hoped there was a cleaner/smaller "toggle" function or alike which I could use but it seems to be related to visibility only - and I want to set a variable (for later usage).

Can this be optimized, if I want a toggle function which should be used to sort a column (and get the value into a variable)?

jsFiddle

<table>
<thead>
  <tr>
    <td>Col 1</td>
    <td>Col 2</td>
    <td>Col 3</td>
  </tr>
</thead>
<tbody>
  <tr>
    <td>A</td>
    <td>B</td>
    <td>C</td>
  </tr>
  <tr>
    <td>B</td>
    <td>C</td>
    <td>A</td>
  </tr>
</tbody>
</table>

And the jQuery stuff:

var column;
var order;

$('thead th').click(function () {

  // Get the current column clicked
  var thisColumn = $(this).text();

  // Check if the column has changed
  if(thisColumn == column) {
    // column has not changed

    if(order == "ascending") {
      order = "descending";
    } else {
      order = "ascending";
    }
  } else {
    // column has changed

    column = thisColumn;
    order = "descending";
  }

  // Replace text in DIV
  $("div").text("column=["+column+"], order=["+order+"]");

  // future code will use the sort order to get database
  // stuff with Ajax

});
\$\endgroup\$
2
  • \$\begingroup\$ You don't have any th elements in your HTML code \$\endgroup\$ Commented Aug 21, 2013 at 17:20
  • \$\begingroup\$ @rink - good point but the th is in the Fiddle demo (I must have copied an old code snippet). \$\endgroup\$ Commented Aug 22, 2013 at 8:39

1 Answer 1

Reset to default
1
\$\begingroup\$

Mostly I'ma java developer, but I think that there are a couple of things that you can do to tidy the code up.

If you do not want to store state in the DOM then you can remove the jQuery data statements and store in the global scope. If I were a jQuery developer I would probably wrap this functionality into a plugin (sortable?) and then to call it on a table with the sortabletable calss use $('.sortableTable').sortable(). It would be more reusable, more encapsulated, and less likely to cause you weird behavior if you have two tables in your page!

However here's my quick take on the above code:

var column;

$('thead th').click(function () {

    // Get the current column clicked
    var thisColumn = $(this);

    if (thisColumn.is(column)) {
        var sort = column.data("sort");
        var newSort = toggle(sort);
        column.data("sort", newSort);
    } else {
        column = thisColumn;
        column.data("sort", false);
    }
    // Replace text in DIV
    $("div").text("column=[" + column.text() + "], order=[" + column.data("sort") + "]");

});

function toggle(current) {
    return !current;
}

You could skip over setting an initial state and have toggle (and you Ajax code) work with undefined values properly (returning false).

\$\endgroup\$
5
  • \$\begingroup\$ I will set your answer as the correct answer as I like your approach on the toggle function. But why use the data("sort",val) approach instead of just setting a variable - what's the catch? \$\endgroup\$ Commented Aug 22, 2013 at 9:10
  • \$\begingroup\$ BTW: I have updated your Fiddle demo here, http://jsfiddle.net/Psz5K/4/ \$\endgroup\$ Commented Aug 22, 2013 at 9:12
  • \$\begingroup\$ Brief to fit in limits! Personally, no catch. Data call uses jQuery's managed storage helps prevent memory leaks. A great way for storing state and behavior against the related elements (maybe a progress bar?). Tracking a single variable(Object) ("column") you can track all important facets ("sort"). Less maintenance to be performed as code gets more complex, fewer errors likely to creep in. Complex plugins can store state within the objects themselves, a more proficient javascript developer can give approaches on that! Whether global, data, revealing module, look into namespacing. \$\endgroup\$ Commented Aug 22, 2013 at 10:28
  • \$\begingroup\$ How about then declaring var column as var global and a global.data("column",$(this)) - would this be any better? \$\endgroup\$ Commented Aug 22, 2013 at 10:54
  • \$\begingroup\$ I think that you shouldn't get hung up on the use of data and consider your question from the point of core javascript development. Take a look at namespacing (one popular syntax you could look for is var myNamespace = myNamespace || {};) Then look for tutorials on the (Revealing) Module Pattern which will help you with encapsulation and protecting your variables - whilst not polluting the global space. One of the joys of javascript is just how many ways you can achieve something, you will find a lot of opinions, many of them will disagree, many of them will be valid. \$\endgroup\$ Commented Aug 22, 2013 at 11:21

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.