Grouping similar jQuery functions

Question

I want to group the two very similar jQuery functions into just one. Any idea?

HTML:

<input id="progress" type="text" class="spinner-input form-control" maxlength="3" readonly value="<?php echo $progreso['progress'] ?>">
<div class="spinner-buttons input-group-btn btn-group-vertical"> 
 <button id="plus" type="button" class="btn spinner-up btn-xs btn-info"> 
 <i class="icon-angle-up"></i> 
 </button> 
 <button id="minus" type="button" class="btn spinner-down btn-xs btn-info"> 
 <i class="icon-angle-down"></i> 
 </button> 
</div>

jQuery:

$(document).on('click', '#plus', function(e){
 var progress = parseInt( $('#progress').val() ) + 5;
 if ( progress <= 100 ) {
 $('#progress').val( progress );
 }
 else{
 $('#progress').val( 100 ); 
 };
});
$(document).on('click', '#minus', function(e){
 var progress = parseInt( $('#progress').val() ) - 5;
 if ( progress >= 0 ) {
 $('#progress').val( progress );
 }
 else{
 $('#progress').val( 0 ); 
 };
});

Answer 1

First step would be to extract a function that updates the input:

function modifyProgress(delta) {
 var element = $('#progress');
 var progress = +element.val() + delta;
 element.val( isNaN(progress) ? 0 : progress < 0 ? 0 : progress > 100 ? 100 : progress;
);
}

BTW, never ever use parseInt with out its second parameter (radix). Some browsers interpret numbers starting with a leading 0 (zero) as octal, so that an input of "09" throws an error and "010" to returns 8. Better is to use the unary plus operator.

This simplifies the event handler:

$(document).on('click', '#plus, #minus', function(e){
 modifyProgress(5 * (this.id === "plus" ? 1 : -1));
})

EDIT: You should consider storing your progress value somewhere other inside the input such as a data model (see MVC). Keeping data/logic separate from the GUI is always a good thing. It also avoids needing to re-parse the number every time. And you can "hide" the checking of overflow or underflow inside the model.

Answer 2

Introduce a third function that both of the first two call.

function adjustSpinner(value, amount) {
 var progress = parseInt(value,10) + amount;
 if (progress > 100) return 100;
 if (progress < 0) return 0;
 return progress;
}
$(document).on('click', '#plus', function(e){
 $('#progress').val(adjustSpinner($('#progress').val(), 5));
});
$(document).on('click', '#minus', function(e){
 $('#progress').val(adjustSpinner($('#progress').val(), -5));
});

• RoToRa: You forgot converting $('#progress').val() to a number. This will just lead to string concatenation.
• Roger: Good point. Got ahead of myself. Fixed.

Answer 3

You could define one function:

function plus_or_minus(event)
{
 var isPlus = $(this).id === '#plus';
 var progress = parseInt($('#progress').val(), 10); 
 if(isPlus)
 progress += 5;
 else
 progress -= 5;
 if(progress > 100)
 progress = 100;
 else if(progress < 0)
 progress = 0;
 $('#progress').val(progress);
}
// And then bind
$(document).on('click', '#plus', plus_or_minus);
$(document).on('click', '#minus', plus_or_minus);

When condensing your code, try to abstract the common parts out to one function - with a simple check against the target element's ID, we can figure out whether to add or subtract. Next, we check progress's value against 100 and 0 to keep it in bounds either way. Finally, apply the value to the #progress element.

Keep in mind that it's a good idea to always pass a base parameter to parseInt - usually the default will be base 10, but there are no promises made when it comes to dealing with as many browsers as there are out there.

PS I haven't tested this, the only part I'm iffy on is the var isPlus = ... as I haven't done jQuery in a while. If that fails to work, maybe try var isPlus = $(this) == $('#plus')

PPS As a bonus, I think jQuery can accept multiple targets for a single on command - research the docs and see if you can bind the function to both elements at the same time

• RoToRa: Checking the id would be id.this === "plus" (without jQuery) or $(this).attr("id") === "plus" (with jQuery). $(this) == $('#plus') just tries to compare two jQuery objects that may both point to the same element, but they still be two different objects and thus never equal.
• phatskat: @RoToRa thanks - I thought maybe the two would be "loosely equal" with the ==