looks fine to me. but in one case, you have function() { and in another, function(){.
best way to figure these things out is to use a checker. for example, I use phpcs for PHP-work, using the Pear coding standard (with some minor modifications)
From the looks of it, the text of the link controls the div that's shown? That seems a little like a case of being cleverer than you need to be. I agree href is better.
不要在不需要的元素上调用 .removeClass，可以节省一些如果您只是执行 $('.nav a.selected').removeClass('selected'); ，或者甚至更好地使用 .siblings ，则无用的内部 DOM 查询如下： $(this).siblings('.selected').removeClass('selected') 这样你就不需要存储另一个对“nav a”的引用，只需让 DOM 来决定什么
just a few suggestions:
do not use the element's inner HTML to decide which tab to show, this will fire back when you want to change the text of the tab, or if you ever need to localize your UI. use a data- attribute instead. $('#main').find($(this).attr('data-tab-id')).show()
use event delegation instead of attaching one event per <a> tag. (use ".delegate" for jQuery < 1.7, or ".on" for the latest jQuery)
do not call .removeClass on elements that do not need it, you can save a few useless internal DOM queries if you just do $('.nav a.selected').removeClass('selected');, or even better use .siblings like this: $(this).siblings('.selected').removeClass('selected') so that you don't need to store yet another reference to "nav a" and simply let the DOM dictate what to do.
//side comments are not really there in prod code, this is just for example purposes
//this code assumes the DOM nodes in main wrapper have no ids and can not get them, obviously ids make a huge difference
$(function () { //make sure the document is ready, not needed if you place in <script> at the bottom
//single var statement validates JSLint
"use strict";
var doc = document,
main = doc.getElementById("main"), //faster than sizzle
$main = $(main), //faster instantiation of jQuery
$wrapper = $main.children(".wrapper"), //faster than find, use if possible
$nav = $(".nav"), //cached
html;
//initialize
$wrapper.hide();
html = $nav.children(".selected").eq(0).html().toLowerCase(); //stop on first
$(doc.getElementById(html)).show();
//add handler
$nav.delegate("a", "click", function () { //delegate outperforms multiple handlers
var $this = $(this); //cached
if ($this.is(".selected")) {
return false; //return early if this is already selected
}
$this.siblings(".selected").eq(0).removeClass("selected"); //you shouldn't need to find more than one
$this.addClass("selected");
$wrapper.hide();
//TODO add ids
$(doc.getElementById($this.html().toLowerCase())).show();
return false;
});
});
None of the selectors are optimized for speed or cached. As jQuery goes, I've seen worse and it will probably work fine. As high-performance, professional JavaScript goes, it gets a D-.
//side comments are not really there in prod code, this is just for example purposes
//this code assumes the DOM nodes in main wrapper have no ids and can not get them, obviously ids make a huge difference
$(function () { //make sure the document is ready, not needed if you place in <script> at the bottom
//single var statement validates JSLint
"use strict";
var doc = document,
main = doc.getElementById("main"), //faster than sizzle
$main = $(main), //faster instantiation of jQuery
$wrapper = $main.children(".wrapper"), //faster than find, use if possible
$nav = $(".nav"), //cached
html;
//initialize
$wrapper.hide();
html = $nav.children(".selected").eq(0).html().toLowerCase(); //stop on first
$(doc.getElementById(html)).show();
//add handler
$nav.delegate("a", "click", function () { //delegate outperforms multiple handlers
var $this = $(this); //cached
if ($this.is(".selected")) {
return false; //return early if this is already selected
}
$this.siblings(".selected").eq(0).removeClass("selected"); //you shouldn't need to find more than one
$this.addClass("selected");
$wrapper.hide();
//TODO add ids
$(doc.getElementById($this.html().toLowerCase())).show();
return false;
});
});
If you want to work on your performance, get very familiar with the work of Nick Zackas and other performance gurus who do presentations and write books on the subject. Rely as little as possible on jQuery. Use JSPerf religiously. Question everything you write and leave comments when you're unsure or think it can be done better in future implementations.
Good luck to you, the results are worth the journey.
发布评论
评论（4）
对我来说看起来不错。但在一种情况下，您有
function() {，而在另一种情况下，您有function(){。解决这些问题的最佳方法是使用检查器。例如，我使用 phpcs 进行 PHP 工作，使用 Pear 编码标准（进行一些小的修改）
looks fine to me. but in one case, you have
function() {and in another,function(){.best way to figure these things out is to use a checker. for example, I use phpcs for PHP-work, using the Pear coding standard (with some minor modifications)
看起来不错。有几点：
从外观上看，链接的文本控制着显示的 div？这似乎有点像你比需要的更聪明的情况。我同意
href更好。我认为这些台词：
初读时有点混乱。像这样的事情：
有点冗长，但也更具可读性（在我看来）。
不要使用
return false来覆盖默认事件行为。使用.preventDefault()（请参阅此处了解更多信息：http://fuelyourcoding.com/jquery-events-stop-misusing-return-false/)Looks good. A few things:
From the looks of it, the text of the link controls the div that's shown? That seems a little like a case of being cleverer than you need to be. I agree
hrefis better.I think the lines:
are a little confusing on first reading. Something like this:
is a little more verbose, but also more readable (in my opinion).
Don't use
return falseto override default event behaviour. Use.preventDefault()instead (see here for more: http://fuelyourcoding.com/jquery-events-stop-misusing-return-false/)只是一些建议：
不要使用元素的内部 HTML 来决定显示哪个选项卡，当您想要更改选项卡的文本或需要本地化 UI 时，这会触发。请改用
data-属性。$('#main').find($(this).attr('data-tab-id')).show()使用事件委托而不是为每个 < 附加一个事件代码> 标签。 （对于 jQuery < 1.7 使用“.delegate”，对于最新的 jQuery 使用“.on”）
不要在不需要的元素上调用
.removeClass，可以节省一些如果您只是执行$('.nav a.selected').removeClass('selected');，或者甚至更好地使用.siblings，则无用的内部 DOM 查询如下：$(this).siblings('.selected').removeClass('selected')这样你就不需要存储另一个对“nav a”的引用，只需让 DOM 来决定什么just a few suggestions:
do not use the element's inner HTML to decide which tab to show, this will fire back when you want to change the text of the tab, or if you ever need to localize your UI. use a
data-attribute instead.$('#main').find($(this).attr('data-tab-id')).show()use event delegation instead of attaching one event per
<a>tag. (use ".delegate" for jQuery < 1.7, or ".on" for the latest jQuery)do not call
.removeClasson elements that do not need it, you can save a few useless internal DOM queries if you just do$('.nav a.selected').removeClass('selected');, or even better use.siblingslike this:$(this).siblings('.selected').removeClass('selected')so that you don't need to store yet another reference to "nav a" and simply let the DOM dictate what to do.所有选择器都没有针对速度进行优化或缓存。随着 jQuery 的发展，我见过更糟糕的情况，但它可能会工作得很好。随着高性能、专业 JavaScript 的发展，它得到了 D-。
如果你想提高自己的表演能力，请非常熟悉尼克·扎卡斯和其他表演专家的工作，他们会就该主题进行演示并撰写书籍。尽可能少地依赖 jQuery。虔诚地使用 JSPerf。对您所写的所有内容提出质疑，并在您不确定或认为在未来的实施中可以做得更好时留下评论。
祝你好运，结果是值得的。
None of the selectors are optimized for speed or cached. As jQuery goes, I've seen worse and it will probably work fine. As high-performance, professional JavaScript goes, it gets a D-.
If you want to work on your performance, get very familiar with the work of Nick Zackas and other performance gurus who do presentations and write books on the subject. Rely as little as possible on jQuery. Use JSPerf religiously. Question everything you write and leave comments when you're unsure or think it can be done better in future implementations.
Good luck to you, the results are worth the journey.