current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I have this jQuery click handler function that has three jobs:

  1. Set the time interval for the refresh rate
  2. Change the color on the clicked refresh rate selection
  3. Callback the tablesort functions to resort the newly loaded data

I'd like to pull out the separate functionality into named functions, but I'm unsure (being a somewhat JS newb) how to do that an achieve the same behavior.

Javascript Code:

$(document).ready(function() {

    $("#refresh-buttons").on("click", "button", function(event) {

        var interval = 0;

        switch(event.target.id)  {
          case "refresh-off" :
                interval = 50000000;
                $(this).parent().children().removeClass("pressed-button");
                $(this).addClass("pressed-button");
                break;

          case "refresh-5-sec" :
              interval = 5000;
              $(this).parent().children().removeClass("pressed-button");
              $(this).addClass("pressed-button");
              break;

          case "refresh-30-sec" :
              interval = 30000;
              $(this).parent().children().removeClass("pressed-button");
              $(this).addClass("pressed-button");
              break;

          case "refresh-60-sec" :
              interval = 60000;
              $(this).parent().children().removeClass("pressed-button");
              $(this).addClass("pressed-button");
              break;
        }

        if (interval != 0)
        {
            clearInterval(intervalId);
            intervalId = setInterval(function(){
                $('#status-tables').load('/dashboard/index #status-tables', function(){
                  $("#workstation-table").tablesorter();
                  $("#report-table1").tablesorter({sortList:[[1,0]]} );
                  $("#report-table2").tablesorter({sortList:[[1,0]]});
                });
            }, interval);
        }

    });
});
share|improve this question

1 Answer 1

up vote 2 down vote

Add the refresh-rates map to #refresh-buttons element ( use .data() function ), optimize amount of $() calls:

$(document).ready(function() {
    var refresh_btns$;

    // you can store id map where ever you prefer to
    // I've figured it would be good to keep them with
    // the element they are related to
    ( refresh_btns$ = $("#refresh-buttons") )
    .data(
        "refreshRates",
        {
            "refresh-off"    :50000000,
            "refresh-5-sec"  :5000,
            "refresh-30-sec" :30000,
            "refresh-60-sec" :60000
        }
    )
    .on("click", "button", function () {
        var
        interval = refresh_btns$.data("refreshRates")[ this.id ] || 0;
        if (
            interval !== 0
        ) {
            // yeah m_x is right about .toggleClass()
            // it producess weird behaviour
            // this shold to the job
            $( this )
            .addClass("pressed-button")
            .siblings()
            .removeClass("pressed-button");

            clearInterval(intervalId);
            intervalId =
            setInterval(
                function () {
                    $('#status-tables')
                    .load(
                        '/dashboard/index #status-tables',
                        function () {
                            $("#workstation-table").tablesorter();
                            $("#report-table1, #report-table2")
                            .tablesorter( { sortList : [ [ 1, 0 ] ] } );
                        }
                    );
                },
                interval
            );
        }
    });
});
share|improve this answer
    
+1 for mapping ids to refresh rates, but 1) wouldn't this toggleClass add pressed-button on all buttons that currently don't have it ? 2) why even use a data-attribute ? your are in a closure, use a simple object in a var. Aside from accessing data attributes, you don't even use your refresh_btns$ var. –  m_x Sep 25 '13 at 20:27

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.