这里有一个更优雅的/DRY/可维护的JS解决方案

A more elegant/DRY/maintainable JS solution here?

本文关键字:维护 JS 解决方案 DRY 有一个 这里      更新时间:2023-09-26

我最初是在编写这段代码时尝试使用JS最佳实践来开始我的任务的,但我很挣扎,所以我只是把它写得一团糟,以使它发挥作用,然后在事后尝试清理它。(你可能会说这不是一个好主意,但这是我目前所能做的最好的事情。)

我的老板建议我尝试使用自定义触发器和事件,但这似乎不是最好的方法。我还在OOJS上读到了丽贝卡·墨菲的这篇文章,这篇文章似乎有答案,但我就是没能做到。

请记住,虽然我肯定会专门寻找一种更优雅(如果依赖于语句,则嵌套更少)的方式来写这篇文章,并且非常感谢在这方面的任何帮助,但我也在寻找可以帮助我解决这里更大的风格缺点并成为一名更好的程序员的模式。例如,我知道这是遍历DOM最笨拙、最慢的方法。

一个重要的注意事项是:在这种情况下,我无法控制大部分标记(除了添加类作为JS/CSS挂钩之外),因为它是由SimpleForm Ruby gem生成的(由于我的分数低,我无法链接到它)。CSS是无关紧要的。所以它实际上只是JS。

上下文

某些内容是根据选择或单选按钮选项显示/隐藏的。

编辑:对不起,我的HTML在这里变得很奇怪,所以我只是做了一个jsFiddle。

编辑2:我们使用的是jQuery 1.6.4,所以如果你想知道为什么我使用.delegate而不是.on,那就是原因。

您还可以通过删除重复的代码来显示/隐藏项目来保持干燥。这里有一个非常简单的方法可以改进:

function changeElements (show_elements, hide_elements) {
    if (show_elements === null) {
        // code to hide all
    }
    container.siblings(show_elements).slideDown();
    container.siblings(hide_elements).slideUp();
}

然后使用该函数,只需给它分别显示和隐藏的选择器:

changeElements('.deceased', '.living, .other-illnesses, .cancer-info');

两件小事(除了提到的两个Alnitak):

1) 不要使用live(请参阅http://bitovi.com/blog/2011/04/why-you-should-never-use-jquery-live.html,或主题上的任何SO帖子)

2) $(document).ready(function(){可以只是$(function(){

除此之外,您的代码并没有什么根本性的问题,老实说,对于这么小的一部分,我认为您可能考虑过度了。如果你对这些元素中的每一个都使用say Backbone视图,那么你可能想切换到事件驱动的方法,但对于像你所拥有的这样小/简单的东西来说,这将是过度的。

有一些显而易见的东西:

  1. 不要重复计算$(this)—只计算一次,并缓存结果
  2. 同样适用于selected.index()
  3. .slideUp.slideDown('fast')使用一个简单的包装器,这样就不会一遍又一遍地重复它们,并且如果需要,可以在一个地方更改动画

最后一个块可以这样写:

var $next = $(this).closest('.control-group').next();
if ($(this).val() === 'true') {
    $next.slideUp();
} else {
    $next.slideDown();
}

事实上,整个if块都可以这样写,尽管有些人可能会发现这有点过于简洁:

$next[$(this).val() === 'true') ? 'slideDown' : 'slideUp']();

几点:

  • 默认情况下应选择无信息选项,而不是空选项。您也没有用于选择那个的处理程序——我建议删除它
  • 在适当的情况下,应该使用id而不是类(在这里:livingdeseasedcancer-infoother-illnessesdiv、deceased-select等)
  • 您真的不应该依赖dom中所选选项的索引——它们可能会更改。相反,使用选定的来确定操作:$(".deceased-select").val()
  • .next()对照组的选择看起来不可靠。最好明确选择你想要的
  • 没有必要在任何地方使用委派事件。用on替换live,用简单的$('.had-cancer-radio').on(…替换delegate
  • 您可以大量减少if else语句:

var value = $(".deceased-select").val(),
    other = $("#other-illnesses, #cancer-info"),
    deceased = container.siblings('.deceased'),
    living = container.siblings('.living');
if (value == "no-information") {
    other.hide();
} else {
    other.slideDown('fast');
    if (value == "living") {
        living.slideDown('fast');
        deceased.hide();
    } else /* if (value == "deceased") */ {
        deceased.slideDown('fast');
        living.hide();
    }
}