Vous rencontrez des difficultés pour appliquer D.R.Y. à mon jQuery

Question

J'ai besoin d'aide pour simplifier mon code. J'essaie d'appliquer le D.R.Y. principe du mieux que je peux et j'ai des problèmes. Toute aide serait grandement appréciée.

$('document').ready(function(){ 
    'use strict'; 

    var body = $('body'); 

    body.find('#button1').click(function(){ 
     body.find('#item1').add('#item2').fadeOut(); 
     body.find('#item3').fadeIn(); 
     body.find('#button1').removeClass('btn-info').addClass('btn-default'); 
     body.find('#button2').removeClass('btn-default').addClass('btn-info'); 
    }); 

    body.find('#button2').click(function(){ 
     body.find('#item3').fadeOut(); 
     body.find('#item1').add('#item2').fadeIn(); 
     body.find('#button2').removeClass('btn-info').addClass('btn-default'); 
     body.find('#button1').removeClass('btn-default').addClass('btn-info'); 
    }); 
}); 

Merci d'avance!

Answer 1

En plus d'utiliser simplement $ pour accéder à des éléments qui sont uniques, vous aimerez peut-être aussi cela comme une amélioration pour la réutilisation et la lisibilité,

function makeInfo(id) { 
    $(id).removeClass('btn-default').addClass('btn-info'); 
} 
function makeDefault(id) { 
    $(id).removeClass('btn-info').addClass('btn-default'); 
} 

$('document').ready(function(){ 
    'use strict'; 

    $('#button1').click(function(){ 
     $('#item1').add('#item2').fadeOut(); 
     $('#item3').fadeIn(); 
     makeInfo('#button2'); 
     makeDefault(this);    
    }); 

    $('#button2').click(function(){ 
     $('#item3').fadeOut(); 
     $('#item1').add('#item2').fadeIn(); 
     makeInfo('#button1'); 
     makeDefault(this);    
    }); 
}); 

Answer 2

Il semble qu'un élément de DRY qui peut être appliqué est pour la partie de la classe:

//add event bound to the click of a element with class btn-default 
body.on('click', '.btn-default', function() { 
    //remove btn-default from this class and add btn-info 
    $(this).removeClass('btn-default').addClass('btn-info'); 
    //remove btn-info from this class and add btn-default 
    $('.btn-info').removeClass('btn-info').addClass('btn-default'); 
}); 

faisant tout le code:

$('document').ready(function(){ 
    'use strict'; 

    var body = $('body'); 

    //add event bound to the click of a element with class btn-default 
    body.on('click', '.btn-default', function() { 
     //remove btn-default from this class and add btn-info 
     $(this).removeClass('btn-default').addClass('btn-info'); 
     //remove btn-info from this class and add btn-default 
     $('.btn-info').removeClass('btn-info').addClass('btn-default'); 
    }); 

    body.find('#button1').click(function(){ 
     body.find('#item1').add('#item2').fadeOut(); 
     body.find('#item3').fadeIn(); 
    }); 

    body.find('#button2').click(function(){ 
     body.find('#item3').fadeOut(); 
     body.find('#item1').add('#item2').fadeIn(); 
    }); 
}); 

Answer 3

Il n'y a pas de répétition inutile, mais il est inutile code. Par exemple, pas besoin de commencer à $('body') et .find() les balises suivantes.

Au lieu de cela, suivez ce modèle:

(1) enlever var body = $('body'); - pas besoin pour elle

(2) Effectuez ces modifications:

body.find('#button1').click(-- can be -- $('#button1').click(

body.find('#item1').add('#item2').fadeOut(); == $('#item1').add('#item2').fadeOut(); 

body.find('#item3').fadeIn(); == $('#item3').fadeIn(); 

etc.

---

Mise à jour: Pour réduire les recherches DOM, mettez en cache l'ensemble du sélecteur:

$('document').ready(function(){ 
    'use strict'; 
    var $i1 = $('#item1'); 
    var $i3 = $('#item3') 
    var $b1 = $('#button1'); 
    var $b2 = $('#button2'); 

    $b1.click(function(){ 
     $i1.add('#item2').fadeOut(); 
     $i3.fadeIn(); 
     $b1.removeClass('btn-info').addClass('btn-default'); 
     $b2.removeClass('btn-default').addClass('btn-info'); 
    }); 

    $b2.click(function(){ 
     $i3.fadeOut(); 
     $i1.add('#item2').fadeIn(); 
     $b2.removeClass('btn-info').addClass('btn-default'); 
     $b1.removeClass('btn-default').addClass('btn-info'); 
    }); 
}); 

Answer 4

Vous pouvez créer une fonction unique (dire « toggleElements ») qui prend un drapeau et effectue toutes les actions nécessaires en fonction de la valeur du drapeau, puis le faire:

body.find('#button1').click(function() { 
    toggleElements(true); 
}); 

Answer 5

Je ne peux pas voir les problèmes dans votre code, et les changements ne permettront pas d'améliorer la lisibilité. Il n'est pas toujours "faire ce que vous pouvez, mais ne répétez pas une seule ligne de code" - vous avez besoin de trouver le meilleur, et votre apparence est bonne.

Cependant, il y a des améliorations possibles:

1. Pourquoi utilisez-vous le corps si vos identifiants d'éléments sont uniques? Vous pouvez simplement utiliser le sélecteur. Vous pouvez combiner le sélecteur au lieu de .add.

Quelque chose comme:

$('document').ready(function(){ 
    'use strict'; 

    $('#button1').click(function(){ 
     $('#item1, #item2').fadeOut(); 
     $('#item3').fadeIn(); 
     $('#button1').removeClass('btn-info').addClass('btn-default'); 
     $('#button2').removeClass('btn-default').addClass('btn-info'); 
    }); 

    $('#button2').click(function(){ 
     $('#item3').fadeOut(); 
     $('#item1,#item2').fadeIn(); 
     $('#button2').removeClass('btn-info').addClass('btn-default'); 
     $('#button1').removeClass('btn-default').addClass('btn-info'); 
    }); 
}); 

Une autre option sont de combiner cette fonctionnalité dans une fonction avec l'argument booléen ou utiliser des classes CSS et transitions. Comme je l'ai dit, à mon avis, 4 lignes de code sont OK pour ne pas être "optimisé". Si vos fonctions grandissent, optimisez-les.

Answer 6

Vous pouvez essayer quelque chose comme ça, où vous définir quels éléments sont pour être affiché en utilisant un attribut data sur chaque bouton. Cela a l'avantage de nécessiter moins de travail lorsque vous avez besoin d'ajouter plus de boutons ou d'éléments, et fonctionne pour n'importe quelle combinaison de boutons/éléments.

jsFiddle

HTML:

<input type="button" id="button1" class="btn-info" value="Button 1" data-show="#item1" /> 
<input type="button" id="button2" class="btn-default" value="Button 2" data-show="#item2" /> 
<input type="button" id="button3" class="btn-default" value="Button 3" data-show="#item2, #item3" /> 

<div id="item1" class="itemClass">Item 1</div> 
<div id="item2" class="itemClass">Item 2</div> 
<div id="item3" class="itemClass">Item 3</div> 

JS:

$('body').on('click', '.btn-default', function() { 
    var show = $(this).data('show'); 
    $('.itemClass').not(show).fadeOut(); 
    $(show).fadeIn(); 

    $('.btn-info').addClass('btn-default').removeClass('btn-info'); 
    $(this).addClass('btn-info').removeClass('btn-default'); 
});