4

以下に示すように、私ははっきりと繰り返します。これは悪い習慣だと理解しています。

では、ifandelseステートメント内の4行の重複するコードを1つにリファクタリングするにはどうすればよいでしょうか。

より良い実践に向けたいくつかのガイダンスをいただければ幸いです。また、このテクニックを学ぶのに役立つと思ったDRYリファレンス/チュートリアル。

$('.inner_wrap .details').click(function() {
    var index = $('.inner_wrap .details').index(this);

    $('.details').removeClass('here');
    $(this).addClass('here');
    $('.view').removeClass('active');
    $(this).find('.view').addClass('active');
    console.log(index);
    if(index > 2){
        index -= 3;
        **// This and its corresponding else statement is the topic of the question**
        $(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide').removeClass('tabShow');
        $(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide:eq(' + index + ')').addClass('tabShow');
    } else {
        $(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide').removeClass('tabShow');
        $(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide:eq(' + index + ')').addClass('tabShow');
    }

    return false;
});
4

5 に答える 5

4

コードは と の両方で繰り返されることに注意してくださいif。つまり、else常に実行されます。ステートメントからそれを取り出してください：

if (index > 2) {
    index -= 3;
}
var elt = $(this).closest('.outer_wrapper').prev('.outer_wrapper');
elt.find('.tabSlide').removeClass('tabShow');
elt.find('.tabSlide:eq(' + index + ')').addClass('tabShow');

次に、jQuery オブジェクトは単なる配列であることに注意してください。次のようにコードを簡素化できます。

if (index > 2) {
    index -= 3;
}
var elt = $(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide');
$(elt.removeClass('tabShow')[index]).addClass('tabShow');

最後に、同じオブジェクトを呼び出す方法を示すためだけに使用される aux 変数を削除できます。

if (index > 2) {
    index -= 3;
}
$($(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide').removeClass('tabShow')[index]).addClass('tabShow');

これを複数行のコードに分割してください:D

[編集]

OK、そして楽しみのために、これはさらに極端な宇宙飛行士タイプのコードで、残りのif.

$($(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide').removeClass('tabShow')[(index <= 2 ? index : index - 3)]).addClass('tabShow');

しかし！それはひどく読めず、IMOです。最初のステップだけに固執する必要があります。パフォーマンスの問題になるまで、無理をしないでください。読みやすさ/保守性を犠牲にして DRY ルールを適用するか、すべてを 1 行のコードにまとめることは、縮小されたコードを読み書きするのと同じくらい理にかなっています。つまり、しないでください:)。

[編集2]

@StuartNelsonは、関数の存在を思い出させてくれました$.eq()。これにより、最終的なコードがこれになります(いくつかの行に分割されます):

$(this).closest('.outer_wrapper')
    .prev('.outer_wrapper')
    .find('.tabSlide')
    .removeClass('tabShow')
    .eq(index <= 2 ? index : index - 3)
    .addClass('tabShow');
于 2012-11-21T15:01:53.133 に答える
2

indexステートメントを使用してのみ変更ifでき、使用されるjQuery要素の変数を保持できます。

$('.inner_wrap .details').click(function() {
    var index = $('.inner_wrap .details').index(this);

    $('.details').removeClass('here');
    $(this).addClass('here');
    $('.view').removeClass('active');
    $(this).find('.view').addClass('active');
    console.log(index);
    if(index > 2){
       index -= 3;
    }
    var element = $(this).closest('.outer_wrapper').prev('.outer_wrapper');
    element.find('.tabSlide').removeClass('tabShow');
    element.find('.tabSlide:eq(' + index + ')').addClass('tabShow');   
    return false;
});
于 2012-11-21T14:53:41.973 に答える
1

あなたが本当に気にかけているのはインデックスを設定することだけなので、それがif staementで必要な唯一のものなので、残りは外に出ることができます：

if(index > 2){
    index -= 3;
}

$(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide').removeClass('tabShow');
$(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide:eq(' + index + ')').addClass('tabShow');
于 2012-11-21T14:55:07.120 に答える
1

これらの 2 行は、if 句と else 句の両方で最後に実行されるため、両方から取り出して、if/else ステートメント全体の後に配置できます。

if(index > 2){
    index -= 3;
}

$(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide').removeClass('tabShow');
$(this).closest('.outer_wrapper').prev('.outer_wrapper').find('.tabSlide:eq(' + index + ')').addClass('tabShow');

このルールは常に真です。それらが if 句と else 句の両方の最初の 2 行である場合は、それらを取り出して、代わりに if ステートメントの前に置きます。

于 2012-11-21T14:52:57.317 に答える
1 
if(index > 2){ index -= 3; }
**// This and its corresponding else statement is the topic of the question**
$(this).closest('.outer_wrapper').prev('.outer_wrapper')
       .find('.tabSlide').removeClass('tabShow')
       .eq(index).addClass('tabShow')

ステートメントに含める必要がないにもかかわらず、そのコードを実行しているため、else を捨てます。tabSlideまた、最初にクラスを持つすべての要素をターゲットにしてから、そのインデックスに基づいてそのクラスの特定のインスタンスだけをターゲットにするため、チェーンで作業を続けることができます。

于 2012-11-21T14:55:21.620 に答える