2012-01-25 5 views
0

У меня есть этот JQuery код:Применяя принцип DRY к этому JQuery код

var thumb = $($('#slider').children('.ui-slider-handle')); 
setLabelPosition();  

$('#slider').bind('slide', function (event, ui) { 
    $('#boksTimer').html((((ui.value)/31) * 60).toFixed(0) + ' min pr. dag'); 
    setLabelPosition(); 
}); 


    function setLabelPosition() { 
    var label = $('#boksTimer'); 
    label.css('top', '10px'); 
    label.css('left', thumb.position().left - (label.width() - thumb.width())/ 2);   
} 


var thumb2 = $($('#slider2').children('.ui-slider-handle')); 
setLabelPosition2();  

$('#slider2').bind('slide', function (event, ui) { 
    $('#boksTimer2').html((((ui.value)/31) * 60).toFixed(0) + ' min pr. dag'); 
    setLabelPosition2(); 
}); 


    function setLabelPosition2() { 
    var label2 = $('#boksTimer2'); 
    label.css('top', '10px'); 
    label.css('left', thumb2.position().left - (label2.width() - thumb2.width())/ 2);   
} 


var thumb3 = $($('#slider3').children('.ui-slider-handle')); 
setLabelPosition3();  

$('#slider3').bind('slide', function (event, ui) { 
    $('#boksTimer3').html((((ui.value)/31) * 60).toFixed(0) + ' min pr. dag'); 
    setLabelPosition3(); 
}); 


    function setLabelPosition3() { 
    var label3 = $('#boksTimer3'); 
    label.css('top', '10px'); 
    label.css('left', thumb3.position().left - (label3.width() - thumb3.width())/ 2);   
} 

Как я могу сделать его более DRY?

DRY = Не повторяйте себе

+4

Это пахнет [обзор кода] (http://codereview.stackexchange.com/) ... – cheeken

+0

Что он должен делать? – j08691

+0

У меня просто куча слайдеров вроде этого: http://jsfiddle.net/z3xV3/50/ 7 –

ответ

4

Вот самый простой способ, которым я могу думать:

$('#slider, #slider2, #slider3').bind('slide', function (event, ui) 
{ 
    var num = this.id.replace('slider',''), 
     $label = $('#boksTimer' + num).html((ui.value/31 * 60).toFixed(0) + ' min pr. dag'), 
     $thumb = $(this).children('.ui-slider-handle'); 

    $label.css({ 
     top: 10, 
     left: $thumb.position().left - ($label.width() - $thumb.width())/2 
    }); 
}); 

Это говорит, вы должны смотреть на изменения кода, чтобы использовать классы.

+0

@Railsbeginner - Я не понимаю, что вы пытаетесь сказать. Код, который я написал, должен работать для всех слайдеров. –

+0

Ваш 'this.id.slice (-1)' не будет работать с идентификатором, который не заканчивается номером. Вы можете сделать это: var num = this.id.replace ('slider', ''); ' –

+1

@amnotiam - Вот почему я использовал' parseInt', так что я уверен, что это число; но я забыл, что я получу «NaN». Я обновил свой ответ. Благодарю. –

1

С одной стороны, вы можете установить параметры в своих функциях - передать в качестве аргументов - вместо того, чтобы просто сделать два одинаковых функции с разными именами.

function setLabelPosition(element) {   
    var label = $(element); 
    label.css('top', '10px'); 
    label.css('left', thumb.position().left - (label.width() - thumb.width())/ 2);   
} 

Но я должен сказать, что вы, вероятно, получите намного более полезный совет, если вы отправили это на участке кода обзора stackexchange: https://codereview.stackexchange.com/

Javascript в вашем сайте, могут расти и расти, пока это только не станет запутанный беспорядок. Вы также должны изучить организационную структуру, такую ​​как Backbone.js, Knockout.js, Sproutcore или Dojo.

Все они могут помочь вам организовать ваш javascript в Модели и Представления.

+0

Не на мета. О кодеревью. –

+0

Yup - опубликовано слишком быстро. Исправлено, спасибо. – PhillipKregg

0

Отделите данные от кода. Создание «Объекты конфигурации», которые могут быть словари, такие как:

// not using javascript syntax but just describing the concept 

cfg1={'name':'slider1', 'timerName':'boksTimer2',} 
cfg2={'name':'slider3', 'timerName':'boksTimer3',} 

function renderCfg(cfg) { 

// use different cfgs here to do whatever you want 

} 
+0

Дайте объяснение тому, кто сделал это. Не помогает плакату или кому-либо не знать причину – Sid

0

Вы можете заменить весь блок кода с этим:

function bindSlider(slider, boks) { 
    var boks$ = $(boks), slider$ = $(slider), thumb$ = slider$.children('.ui-slider-handle'); 
    slider$.bind('slide', function (event, ui) { 
     boks$.html((((ui.value)/31) * 60).toFixed(0) + ' min pr. dag') 
      .css({'top': '10px', 'left', thumb$.position().left - (boks$.width() - boks$.width())/ 2}); 
    }); 
} 

bindSlider('#slider', '#boksTimer'); 
bindSlider('#slider2', '#boksTimer2'); 
bindSlider('#slider3', '#boksTimer3'); 

Или, еще более лаконично, как это:

$('#slider, #slider2, #slider3').bind('slide', function (event, ui) { 
    var boks$ = $("#boksTimer" + this.id.replace("slider", "")); 
    boks$.html((((ui.value)/31) * 60).toFixed(0) + ' min pr. dag') 
     .css({'top': '10px', 'left', $(this).children('.ui-slider-handle').position().left - (boks$.width() - boks$.width())/ 2}); 
}); 
Смежные вопросы