JQuery drying function

Question

As you can see below, I repeat clearly. I understand that this is bad practice.

So how do four repeating lines of code inside an if and else statement reorganize into one?

It would be very helpful to evaluate some of the best practice recommendations. In addition, any DRY links / manuals that you find useful in learning this technique.

$('.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; }); 

Answer 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') 

Turn else, as you are executing this code, no matter what you do not need to include it in the statement. You can also continue to work on the chain, since you first target all the elements with the tabSlide class, and then target only a specific instance of this class based on its index.

Answer 2

Note that the code repeats in both if and else , that is, it always executes. Just make it out of the statement:

 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'); 

Then notice that the jQuery object is just an array. You can simplify your code this way:

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

Finally, we can exclude the aux variable, used just to demonstrate how you call the same object:

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

Please split this into several lines of code: D

[EDIT]

Well, and just for fun, here is an even more extreme code like an astronaut, getting rid of the remaining if :

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

BUT! This is terribly unreadable and IMO, you should only stick to the first step. While this is not a performance issue, do not overdo it. Applying the DRY rule due to readability / maintainability or just inserting everything into one line of code makes the same sense as reading and writing abbreviated code. That is, do not do this :).

[EDIT 2]

@StuartNelson reminded me of the existence of the $.eq() function, which would lead to this final code (split into several lines):

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

Answer 3

You can only change index with the if , and you can save the variable for the jQuery element used:

 $('.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; }); 

Answer 4

These two lines are executed last in both if and else cases, so they can be pulled out of both and placed after the entire if / else statement:

 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'); 

This rule is always true. If they were the first two lines in your if and else clause, you would pull them out and put them in front of your if statement.

Answer 5

Since all you really need is an index setting, this is the only thing you need in case the rest can come out:

 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');