我的jQuery代码正在运行,但从程序员的角度来看它是非常糟糕的吗？

Question

我拼凑了这个jQuery函数.它的目的是计算内部所有img元素的边距,div.article以便平衡图像的高度与文档的基线网格,为20像素.为了匹配我的基线网格,每个图像高度应为20的倍数.如果不是这种情况,例如一个图像的高度为154像素,则该函数会为img添加6 px的余量,以便与基线平衡网格已恢复.

该函数正常工作,所以我的实际问题是:由于我不是程序员,我想知道我的代码是否非常糟糕,虽然它正在工作,如果是这样,代码如何改进？

jQuery代码:

$('div.article img').each(function() {

    // define line height of CSS baseline grid:
    var line_height = 20;

    // capture the height of the img:
    var img_height = $(this).attr('height');

    // divide img height by line height and round up to get the next integer:
    var img_multiply = Math.ceil(img_height / line_height);

    // calculate the img margin needed to balance the height with the baseline grid:
    var img_margin = (img_multiply * line_height) - img_height;

    // if calculated margin < 5 px:
    if (img_margin < 5) {

        // then add another 20 px to avoid too small whitespace:
        img_margin = img_margin + 20;  
    }

    // if img has caption:
    if ($($(this)).next().length) {

        // then apply margin to caption instead of img:
        $($(this)).next().attr('style', "margin-bottom: " + img_margin + "px;");
    } else {

        // apply margin to img:
        $(this).attr('style', "margin-bottom: " + img_margin + "px;");
    }
});

HTML代码示例,带标题的img:

<div class="article">
   <!-- [...] -->
    <p class="image">
        <img src="http://example.com/images/123.jpg" width="230" height="154" alt="Image alt text goes here" />
        <small>Image Caption Goes Here</small>
    </p>
   <!-- [...] -->
</div>

HTML代码示例,img没有标题:

<div class="article">
    <!-- [...] -->
    <p class="image">
        <img src="http://example.com/images/123.jpg" width="230" height="154" alt="Image alt text goes here" />
    </p>
   <!-- [...] -->
</div>

编辑:基于Russ Cam建议的精炼代码:

var line_height = 20;
var min_margin = 5;
$('div.article img').each(function() {
    var $this = $(this); // prefixed variable name with $ to remind it's a jQuery object
    var img_height = $this.height();
    var img_margin = ((Math.ceil(img_height / line_height)) * line_height) - img_height;
    img_margin = (img_margin < min_margin)? img_margin + line_height : img_margin;
    if ($this.next().length) {      
        $this.next().css({'margin-bottom' : img_margin + 'px'});
    } else {
        $this.css({'margin-bottom' : img_margin + 'px'});
    }
});

Answer 1

我可以推荐一些改进

1.cache $(this)在each()局部变量中

$('div.article img').each(function() {

    var $this = $(this); // prefixed variable name with $ 
                         // to remind it's a jQuery object

    // ....

   if ($this.next().length) {

   // ....

   }

});

2.而不是设置attr('style'),使用css()命令

3.不需要这样做

$($(this))

虽然它不会破坏jQuery,但是将jQuery对象传递给另一个jQuery对象是不必要的.

4.使用$(this).height() 或$(this).outerHeight()获取浏览器中元素的高度

5.没有jQuery特定,但可以使用三元条件运算符来分配此值

// if calculated margin < 5 px:
if (img_margin < 5) {

    // then add another 20 px to avoid too small whitespace:
    img_margin = img_margin + 20;  
}

变

// if calculated margin < 5 px then add another 20 px 
// to avoid too small whitespace
img_margin = (img_margin < 5)? img_margin + 20 : img_margin;  

正如亚历克斯在评论中指出的那样,我还会删除那些仅仅重复代码告诉你的多余评论.即使这是调试脚本,在我看来,它们会降低可读性,注释应仅用于提供读取代码时不具备的细节.