jQuery - Improving code and eliminating repetition

Question

I have a series of calls to various jQuery methods and I was just wondering if there was a way to clean this code up, turn it into a function that I pass the selectors into, or anything else that could make the the code more manageable/maintainable. What I have is a very long list of lines of code like these with a lot of repetition:

...
$('#boxcolor').on('change', function() {
    $('header').css('background-color', $('#boxcolor').val());
});
$('#color').on('change', function(){
    $('header').css('color',$('#color').val());
});
$('#size').keyup(function() {
    $('header').css('font-size', $('#size').val() + 'px');    
});
...

Any suggestions would be great, thanks.

I don't see much actual repetition in this particular snipet. Are you doing the same exact set of things to more elements than "header"? Generally I see a tradeoff between lots of refactoring to make code small and readability -- some spacing things out can help you later when maintaining the code. — Devin Ceartas, Apr 16 '13 at 03:01

gustavohenke · Answer 1 · 2013-04-16T03:10:00.603

5

Just my suggestion:

var header = $('header');

$('#boxcolor').change({
    property: 'background-color'
}, handler );

$('#color').change({
    property: 'color'
}, handler );

$('#size').keyup({
    property: 'font-size',
    unit: 'px'
}, handler );

function handler( e ) {
    header.css( e.data.property, $( this ).val() + ( e.data.unit || "" ) );
}

edited Apr 16 '13 at 03:10

answered Apr 16 '13 at 03:04

gustavohenke

40,997
14
121
129

I like this idea, but in my opinion is less readable than the original one. Still +1 – nicosantangelo Apr 16 '13 at 03:07
Just some formatting in those objects, and ta-da! – gustavohenke Apr 16 '13 at 03:08
Nice way of using data transfer :) Unfortunately this will lock you into only being able to make css changes, unless you wrap *that* as well. – Ja͢ck Apr 16 '13 at 03:51

score 0 · Answer 2 · answered Apr 16 '13 at 03:03

I don't see a lot of things you could refactor, but, if you are always selecting the header, you could do:

var changeHeaderStyles = function() {
   var header = $('header');
   //use a clousure here to only get the header once
   return function(cssObject) {
       $header.css(cssObject);
   }
}(); 

//Then..
$('#size').keyup(function() {
    changeHeaderStyles({ 'font-size': $('#size').val() + 'px' });    
});

So then you could change more than one style at once, and you can read clearly what is happening.

Hope it helps

jQuery - Improving code and eliminating repetition

2 Answers2