重构代码

比方说，我有以下代码：重构代码

$(function() { 
    $(".buy-it-now.ribbon").click(function() { 
     $(".bid-to-beat.ribbon.active").removeClass("active"); 
     $(".bid-to-beat.ribbon").addClass("inactive"); 
     $(".buy-it-now.ribbon.inactive").removeClass("inactive"); 
     $(".buy-it-now.ribbon").addClass("active"); 
     $(".bid-now").hide(); 
     $(".buy-now").show(); 
     $(".add-to-cart").hide(); 
    }) 
    $(".bid-to-beat.ribbon").click(function() { 
     $(".buy-it-now.ribbon.active").removeClass("active"); 
     $(".buy-it-now.ribbon").addClass("inactive"); 
     $(".bid-to-beat.ribbon").removeClass("inactive"); 
     $(".bid-to-beat.ribbon").addClass("active"); 
     $(".buy-now").hide(); 
     $(".bid-now").show(); 
     $(".add-to-cart").show(); 
    }); 
});

这是一个简单的功能，允许多个用户界面相关的事情发生在我工作的网站的前端。我对jQuery和JavaScript一般都很新，并且正在学习重构，并且现在让我的代码更加简洁。我目前编写代码的方式是每个想法都有一定的路线。所以我的问题是一个有经验的开发人员如何编写这个相同的代码或者说，我怎么能重构这段代码？

来源

2012-09-12 Dain

尝试以下操作：

$(function() { 
    var $handlers = $('.buy-it-now.ribbon, .bid-to-beat.ribbon'); 

    $handlers.click(function() { 
     $handlers.toggleClass("active inactive"); 

     var $elements = $(".bid-now, .add-to-cart"), 
      $buyElement = $(".buy-now"); 

     if($(this).is('.buy-it-now.ribbon')) { 
      $elements.hide(); 
      $buyElement.show(); 
     } else { 
      $elements.show(); 
      $buyElement.hide(); 
     } 
    }); 
});

来源

2012-09-12 20:15:42

哇，真是太棒了。这完全打乱了我的想法。感谢您的洞察力。 – Dain

@Dain不客气。 –

@RicardoLohmann，这很酷..从来没有想过这种方法！ –

这个问题会更适合codereview，但是是的，它可以使用方法链接浓缩一点。

$(function() { 
    $(".buy-it-now.ribbon").click(function() { 
     $(".bid-to-beat.ribbon").removeClass("active").addClass("inactive"); 
     $(".buy-it-now.ribbon").removeClass("inactive").addClass("active"); 
     $(".bid-now").hide(); 
     $(".buy-now").show(); 
     $(".add-to-cart").hide(); 
    }) 
    $(".bid-to-beat.ribbon").click(function() { 
     $(".buy-it-now.ribbon").removeClass("active").addClass("inactive"); 
     $(".bid-to-beat.ribbon").removeClass("inactive").addClass("active"); 
     $(".buy-now").hide(); 
     $(".bid-now").show(); 
     $(".add-to-cart").show(); 
    }); 
});

您可以通过预先选择的元素，并且只要没有元素在页面的生命周期中添加或移除的单击事件之前缓存他们变量进一步压缩它。

来源

2012-09-12 19:53:00

啊，谢谢你的建议。不知道codereview。 – Dain

您可以使用'toggleClass'将其组合更多。 '$（“。buy-it-now.ribbon”）。toggleClass（“active inactive”）;'。你可以通过结合选择器来结合它：'$（“。bid-now，.add-to-cart”）。show（）'。 –

我想过切换类，但不会按预期工作吗？积极的人会变得不活跃，所有不活跃的人将变得活跃，而不是全部变得不活跃。我同意结合选择器，除非你遇到IE6/7的性能问题（不知道是否曾经修复过） –

作为代码，您可以将一些选择器合并到一行中。还因为你的元素看起来是静态的，你可以将它们缓存到一个变量中，稍后使用它们，因为它减少了在DOM中查找元素的次数，从而减少了访问时间。

此外，您可以限制这些变量或选择通过在物体或封闭装箱他们的范围..

或许真的在这行..

 $(function() { 
    cart.init(); 
}); 

var cart = { 
    elems : { 
     $buyRibbon : null, 
     $bidRibbon : null, 
     $bidNow: null, 
     $buyNow: null, 
     $addToCart: null 
    }, 
    events : { 
    }, 
    init : function() { 
     this.elems.$buyRibbon = $(".buy-it-now.ribbon"); 
     this.elems.$bidRibbon = $(".bid-to-beat.ribbon"); 
     this.elems.$bidNow = $(".bid-now") ; 
     this.elems.$buyNow = $(".buy-now") ; 
     this.elems.$addToCart = $(".add-to-cart") ; 
     this.events.buyClick(); 
     this.events.bidClick(); 
    } 
}; 

cart.events.buyClick = function() { 
    cart.elems.$buyRibbon.on('click', function(){ 
     cart.elems.$bidRibbon.removeClass('active').addClass('inactive'); 
     cart.elems.$buyRibbon.removeClass('inactive').addClass('active'); 
     cart.elems.$bidNow.hide(); 
     cart.elems.$buyNow.show(); 
     cart.elems.$addToCart.hide(); 
    }); 
} 

cart.events.bidClick = function() { 
    cart.elems.$bidRibbon.on('click', function(){ 
     cart.elems.$buyRibbon.removeClass('active').addClass('inactive'); 
     cart.elems.$bidRibbon.removeClass('inactive').addClass('active'); 
     cart.elems.$bidNow.show(); 
     cart.elems.$buyNow.hide(); 
     cart.elems.$addToCart.show(); 
    }); 
}

所以在这里基本上你的整个车是一个对象..和车具有与此相关的不同属性..您在此遵循面向对象编程的原则。个使用闭包，我听到让你更好的设计限制你的代码的范围..

来源

2012-09-12 20:13:51

哇。这令人难以置信的印象深刻。我很感激你花时间向我展示这一点。我想我会用类似的东西，因为它可以让我在将来有很多的可重用性。 – Dain

嗯。我想使用此代码，但它不起作用。：/不知道如何解决它，不够先进。 – Dain

您是否在浏览器的控制台部分看到任何错误？ –

我可以建议是这样的：

$(function() { 
    var buyNowButton = $('buy-it-now.ribbon'), 
     bidToBeatButton = $('.bid-to-beat.ribbon'), 
     buyNowEls = $('.buy-now'), 
     bidToBeatEls = $('.bid-now,.add-to-cart'); 

    var toggleButtons = function(showBuyNow){ 
     buyNowButton.toggleClass('active', showBuyNow); 
     bidToBeatButton.toggleClass('active', !showBuyNow); 
     buyNowEls.toggle(showBuyNow); 
     bidToBeatEls.toggle(!showBuyNow); 
    } 

    buyNowButton.click(function(){ toggleButtons(true) }); 
    bidToBeatButton.click(function(){ toggleButtons(false) }); 
});

可以为您节省在开始时删除选择器，并在恰当的位置进行选择，如果保存的空间比次要的性能命中更重要，则需要一些行。然后它看起来像这样：

$(function() { 
    var toggleButtons = function(showBuyNow){ 
     $('buy-it-now.ribbon').toggleClass('active', showBuyNow); 
     $('.bid-to-beat.ribbon').toggleClass('active', !showBuyNow); 
     $('.buy-now').toggle(showBuyNow); 
     $('.bid-now,.add-to-cart').toggle(!showBuyNow); 
    } 

    $('buy-it-now.ribbon').click(function(){ toggleButtons(true) }); 
    $('.bid-to-beat.ribbon').click(function(){ toggleButtons(false) }); 
});

第一个版本选择一个元素并将它们保存在内存中;第二个按钮每次点击按钮时选择它们。两者都解决了我认为会出现的问题，在选定的答案中点击两次相同的按钮会导致.active和.inactive类与显示/隐藏元素不同步。

来源

2012-09-12 21:15:43 undefined

回答

相关问题