Рефакторинг функций в jQuery

Question

Я совершенно новый с JQuery, но Google в настоящее время является моим лучшим другом с конструированием кода. Код ниже немного длинный, и мне интересно, если a) я сделал это правильно (все работает отлично) и б) есть ли лучший способ сделать это? Я уверен, что есть более простые методы для достижения того, что мне нужно, но код, который я собрал, делает то, что ему нужно. Полагаю, я просто прошу критиковать то, что я узнал.

$(window).load(function() { 
    window.$order = "GetData.asp"; 
    GetData(); 
    window.$ord == ''; 
    $("#descn").hide();   
    $("#desc").hide();   
    $("#desct").hide();   
}); 
$("#asc").click(function(){ 
    window.$order = "GetData.asp?order=asc"; 
    GetData(); 
    $("#desc").show(); 
    $("#asc").hide(); 
}); 
$("#desc").click(function(){ 
    window.$order = "GetData.asp?order=desc" 
    GetData(); 
    $("#desc").hide(); 
    $("#asc").show(); 
}); 
$("#ascn").click(function(){ 
    window.$order = "GetData.asp?no=asc"; 
    GetData(); 
    $("#descn").show(); 
    $("#ascn").hide(); 
}); 
$("#descn").click(function(){ 
    window.$order = "GetData.asp?no=desc" 
    GetData(); 
    $("#descn").hide(); 
    $("#ascn").show(); 
}); 
$("#asct").click(function(){ 
    window.$order = "GetData.asp?test=asc"; 
    GetData(); 
    $("#desct").show(); 
    $("#asct").hide(); 
}); 
$("#desct").click(function(){ 
    window.$order = "GetData.asp?test=desc" 
    GetData(); 
    $("#desct").hide(); 
    $("#asct").show(); 
}); 
function GetData() { 
    $.getJSON(window.$order, function(data){ 
    if (data[0].name == 'none') { 
     $('#output').html("<div id='alert'>None Available</div>"); 
    } else { 
     var len = data.length; 
     html = '<table class="tbl small">' 
     $time = 0 
     for (var i = 0; i< len; i++) { 

      $time = (parseInt($time)+parseInt(data[i].time)); 
      if (data[i].overdue == "Yes") { 
       html = html + '<tr style="background: #FF0000">' 
      } else if (data[i].status == "RE ASSES") { 
       html = html + '<tr style="background: #D0C0C0">' 
      } else{ 
       html = html + '<tr>' 
      } 
      html = html + '<td class="border col1">'+data[i].line+'</td>' 
      html = html + '<td class="border col2">'+data[i].no+'</td>' 
      html = html + '<td class="border col3">'+data[i].desc+'</td>'   
      html = html + '<td class="border col4">'+data[i].loc+'</td>'    
      html = html + '<td class="border col5">'+data[i].time+'</td>'   
      html = html + '<td class="border col6">'+data[i].lastcal+'</td>'    
      html = html + '<td class="border col7">'+data[i].frequency+'</td>'   
      html = html + '<td class="border col8">'+data[i].status+'</td>' 
      if (data[i].external == "E") { 
       html = html + '<td class="border col9">&#10004;</td>' 
      } else { 
       html = html + '<td class="border col9"></td>' 
      } 
      html = html + '</tr>'   
      } 
     html = html + '</table>' 
     $('#output').html(html); 
     var monthNames = ["January", "February", "March", "April", "May", "June", 
      "July", "August", "September", "October", "November", "December" 
     ]; 
     var d = new Date(); 
     $('#tottime').html('Total time outstanding for ' + monthNames[d.getMonth()] + ' : ' + $time); 
    } 
    }); 
} 

Моя единственная забота в том, что я делаю вещи длинного путь вокруг, например, функции #asc клик, например, - я не уверен, что лучшим способом достижения этой цели без использования функции для каждого методов щелчка.

Также; внутри цикла for мне пришлось вручную установить цвет фона с помощью if else, поскольку просто попытка использовать .css ('background', 'red') не работает, но то, что я здесь сделал.

Большое спасибо за ваше время и, пожалуйста, не стесняйтесь указать на какие-либо проблемы.

Answer 1

Вы можете свести к минимуму код, используя свойство HTML5 data. Например,

<a href="javascript:;" class="order-cls" data-order="asc" data-hide-property="descn" data-show-property="ascn">Link</a> 

А в JS файл

$(function(){ 
    $('a.order-cls').click(function(){ 
    var me = $(this), 
     order = me.data('order'), 
     hideprop = me.data('hideProperty'), 
     showprop = me.data('showProperty'); 

     window.$order = "GetData.asp?no="+order; 
     GetData(); 
     $("#"+showprop).show(); 
     $("#"+hideprop).hide(); 
    }); 
}); 

надеюсь, что это поможет вам.

Answer 2

Существует лучший способ сделать это, и это уже описано Sagar (используйте атрибуты данных html5).

Это может быть сделано без ID селекторов, в то время как URL не должен быть передан GetData() через глобальную переменную, но с помощью параметра: GetData(url);

Во всяком случае, здесь сокращенный вариант кода .. . Является ли это лучше ... Нах, если вы не писали сами, что это довольно вероятно, нечитаемым: D

var params = { 
    "": 'order', 
    "n": 'no', 
    "t": 'test' 
}; 

$('#asc, #desc, #ascn, #descn, #asct, #desct').on('click', function(event){ 
    var $this = $(this), 
     id = $this.attr('id'), 
     order = id.substr(0,1) == 'a' ? id.substr(0,3) : id.substr(0,4), // asc or desc 
     type = id.substr(type=='asc'?3:4,1), // "", "t" or "n" 
     param = params[type]; 
    window.$order = ['GetData.asp?',param,'=',order].join(''); 
    GetData(); 
    $('#desc' + type)[type=='asc'?'show':'hide'](); 
    $('#asc' + type)[type=='desc'?'show':'hide'](); 
}); 

Answer 3

Я не знаю, если вам нужен .load() метод для него, потому что он ждет, чтобы все было загружено первым. Вместо JQuery имеет метод, называемый .ready(), который просто нуждается в DOM (Document Object Model) быть готовым, так что вы можете использовать это, и вы можете использовать рефакторинга это следующим образом:

var myApp = myApp || {}; // use this to stop polluting the global scope 
    myApp.GetData = function(){ 
     $.getJSON(myApp.$order, function(data){ 
      if (data[0].name == 'none') { 
       $('#output').html("<div id='alert'>None Available</div>"); 
      } else { 
       var len = data.length, 
       html = '<table class="tbl small">', 
       $time = 0; 
       for (var i = 0; i< len; i++) { 
        $time = (parseInt($time)+parseInt(data[i].time)); 
        if (data[i].overdue == "Yes") { 
         html = html + '<tr style="background: #FF0000">' 
        } else if (data[i].status == "RE ASSES") { 
         html = html + '<tr style="background: #D0C0C0">' 
        } else{ 
        html = html + '<tr>' 
        } 
       html = html + '<td class="border col1">'+data[i].line+'</td>' 
       html = html + '<td class="border col2">'+data[i].no+'</td>' 
       html = html + '<td class="border col3">'+data[i].desc+'</td>'   
       html = html + '<td class="border col4">'+data[i].loc+'</td>'    
       html = html + '<td class="border col5">'+data[i].time+'</td>'   
       html = html + '<td class="border col6">'+data[i].lastcal+'</td>'    
       html = html + '<td class="border col7">'+data[i].frequency+'</td>'   
       html = html + '<td class="border col8">'+data[i].status+'</td>' 
       if (data[i].external == "E") { 
        html = html + '<td class="border col9">&#10004;</td>' 
       } else { 
        html = html + '<td class="border col9"></td>' 
       } 
        html = html + '</tr>'   
       } 
        html = html + '</table>' 
        $('#output').html(html); 
        var monthNames = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"]; 
        var d = new Date(); 
        $('#tottime').html('Total time outstanding for ' + 
          monthNames[d.getMonth()] + ' : ' + $time); 
       } 
     }); 
    }; 

$(function(){ //<----shorter way of writing $(document).ready(fn) 
    // Note: you should bind your events like click, change etc in this block 
    myApp.$order = "GetData.asp"; // now you can put in your custom var myApp 
    myApp.GetData(); 
    myApp.$ord == ''; 
    $("#descn, #desc, #desct").hide(); // you can use this way to hide but 
             // better to hide with css like 
             // #descn, #desc, #desct{display:none;} 

    $("#asc, #desc, #ascn, #descn, #asct, #desct").click(function(){ 
     if(this.id === "asc"){ 
      myApp.$order = "GetData.asp?order=asc"; 
      $("#desc").show(); 
      $("#asc").hide(); 
     }else if(this.id === "desc"){ 
      myApp.$order = "GetData.asp?order=desc"; 
      $("#desc").hide(); 
      $("#asc").show(); 
     }else if(this.id === "ascn"){ 
      myApp.$order = "GetData.asp?no=asc"; 
      $("#descn").show(); 
      $("#ascn").hide(); 
     }else if(this.id === "descn"){ 
      myApp.$order = "GetData.asp?no=desc"; 
      $("#descn").hide(); 
      $("#ascn").show(); 
     }else if(this.id === "asct"){ 
      myApp.$order = "GetData.asp?test=asc"; 
      $("#desct").show(); 
      $("#asct").hide(); 
     }else if(this.id === "desct"){ 
      myApp.$order = "GetData.asp?test=desc"; 
      $("#desct").hide(); 
      $("#asct").show(); 
     } 

     myApp.GetData(); // <---you can call this function here now. 

    }); 

});