skip to Main Content

Today, I code some JavaScript, but it really does not look good. So how can I optimize it with good extend and look delicious

if($("#active-flg").val() == 'clubInfo') {
    $("[name='local-li']:eq(0)").addClass("active");
    $("[name='local-li']:eq(1)").removeClass("active");
    $("[name='local-li']:eq(2)").removeClass("active");

    $("#manage-club-info").addClass("active");
    $("#manage-club-config").removeClass("active");
    $("#manage-club-phone").removeClass("active");

} else if($("#active-flg").val() == 'clubSeo') {
    $("[name='local-li']:eq(0)").removeClass("active");
    $("[name='local-li']:eq(1)").addClass("active");
    $("[name='local-li']:eq(2)").removeClass("active");

    $("#manage-club-info").removeClass("active");
    $("#manage-club-seo").addClass("active");
    $("#manage-club-phone").removeClass("active");

} else if($("#active-flg").val() == 'clubPhone') {
    $("[name='local-li']:eq(0)").removeClass("active");
    $("[name='local-li']:eq(1)").removeClass("active");
    $("[name='local-li']:eq(2)").addClass("active");

    $("#manage-club-info").removeClass("active");
    $("#manage-club-seo").removeClass("active");
    $("#manage-club-phone").addClass("active");
}

4

Answers


  1. Try this..

    if($("#active-flg").val() == 'clubInfo') {
        removeClass(["[name='local-li']:eq(1)","[name='local-li']:eq(2)","#manage-club-config","#manage-club-phone"]);
        addClass(["[name='local-li']:eq(0)","#manage-club-info"])
    
    } else if($("#active-flg").val() == 'clubSeo') {
        removeClass(["[name='local-li']:eq(0)","[name='local-li']:eq(2)","#manage-club-info","#manage-club-phone"]);
        addClass(["[name='local-li']:eq(1)","#manage-club-seo"])
    
    } else if($("#active-flg").val() == 'clubPhone') {
        removeClass(["[name='local-li']:eq(0)","[name='local-li']:eq(1)","#manage-club-info","#manage-club-seo"]);
        addClass(["[name='local-li']:eq(2)","#manage-club-phone"]);
    }
    
    
    function addClass(arraySelectors){
        for (selector in arraySelectors){
            $(arraySelectors[selector]).addClass("active");
        }
    }
    
    function removeClass(arraySelectors){
        for (selector in arraySelectors){
            $(arraySelectors[selector]).removeClass("active");
        }
    }
    
    Login or Signup to reply.
  2. What about this

    var actions = {
        clubInfo: function(){
            $("[name='local-li']:eq(0)").addClass("active");
            $("[name='local-li']:eq(1)").removeClass("active");
            $("[name='local-li']:eq(2)").removeClass("active");
    
            $("#manage-club-info").addClass("active");
            $("#manage-club-config").removeClass("active");
            $("#manage-club-phone").removeClass("active");
    
        },
        clubSeo: function(){
            $("[name='local-li']:eq(0)").removeClass("active");
            $("[name='local-li']:eq(1)").addClass("active");
            $("[name='local-li']:eq(2)").removeClass("active");
    
            $("#manage-club-info").removeClass("active");
            $("#manage-club-seo").addClass("active");
            $("#manage-club-phone").removeClass("active");
        },
        clubPhone: function(){
            $("[name='local-li']:eq(0)").removeClass("active");
            $("[name='local-li']:eq(1)").removeClass("active");
            $("[name='local-li']:eq(2)").addClass("active");
    
            $("#manage-club-info").removeClass("active");
            $("#manage-club-seo").removeClass("active");
            $("#manage-club-phone").addClass("active");
        }
    }
    var action = $("#active-flg").val();
    if(actions[action]) {
        actions[action]();
    } else {
        sexyDefaultFunction();
    }
    
    Login or Signup to reply.
  3. While this is not code review,

    var flags = ['clubInfo', 'clubSeo', 'clubPhone']
    var targets = ['#manage-club-info','#manage-club-seo','#manage-club-phone'] 
    var val = $("#active-flg").val();
    var index = flags.indexOf(val);
    
    if (index >= 0){
        $("[name='local-li']").removeClass("active");
        $(targets.join(',')).removeClass("active");
    
        var activeLi = "[name='local-li']:eq("+ index +")";
        $(activeLi).addClass("active");
        $(targets[index]).addClass("active");    
    }
    
    Login or Signup to reply.
  4. Your code is multiple kinds of redundant, which isn’t good. One of the most fundamental reasons computers exist is to eliminate redundancy.

    What you want to be doing is finding ways to iterate over the things you have recursively and leaving the redundancy up to the computer or better yet just sidestepping redundancy and only dealing with the values that you need to. AKA instead of:

    if(var == "bread") {
        $("#bread").addClass("active");
    }
    
    if(var == "milk") {
        $("#milk").addClass("active");
    }
    
    if(var == "eggs") {
        $("#eggs").addClass("active");
    }
    

    You want something like:

    $("#" + var).addClass("active");
    
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search