HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Price Slider using jQuery UI working in Java environment to be optimised

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
priceoptimisedworkingsliderjavaenvironmentjqueryusing

Problem

I am working on a price slider that is for a Java environment. It is working correctly but I would like to optimise it as much as possible. Any tips/advice?

```
















var slider_step = $(".slider_step"),
price_min = $(".price_min"),
price_max = $(".price_max"),
slider_min = $(".slider_min"),
slider_max = $(".slider_max");

$( ".range" ).slider({
range: true,
min: parseInt(price_min.val()),
max: parseInt(price_max.val()),
step: parseInt(slider_step.val()),
values: [ parseInt(slider_min.val()), parseInt(slider_max.val()) ],
slide: function(event, ui){
//Sliding number
$('.ui-slider-handle:eq(0) .price-range-min').html('£' + ui.values[0]);
$('.ui-slider-handle:eq(1) .price-range-max').html('£' + ui.values[1]);
$('.price-range-both').html('£' + ui.values[0] + ' - £' + ui.values[1] );

if ( ui.values[0] == ui.values[1] ){
$('.price-range-both i').css('display', 'none');
} else{
$('.price-range-both i').css('display', 'inline');
}
if (collision($('.price-range-min'), $('.price-range-max')) == true){
$('.price-range-min, .price-range-max').css('opacity', '0');
$('.price-range-both').css('display', 'block');
} else{
$('.price-range-min, .price-range-max').css('opacity', '1');
$('.price-range-both').css('display', 'none');
}
},
change: function(event, ui){
/ update the values and submit the form /
slider_min.val(ui.values[0]);
slider_max.val(ui.values[1]);

// getting search query string from html
var q = $(".slider_q");

// create string from array and add priceValue and prices in required format.
var qrystr = q.val();
if (qrystr.indexOf("priceValue") == -1){
var r = qrystr+":priceValue:["+

Solution

From a once over;

  • You are assigning a class to hidden input elements, that does not make sense. Assign an id instead, selection on id is much faster.



  • ui.values[0] and ui.values[1] use magic sonstants, you should use named constants like ui.values[SLIDER_MINIMUM] and ui.values[SLIDER_MAXIMUM]



  • Do not disemvowel your variable too much , qrystr



  • You declare var r more than once in change, don't do that



  • The variable name in collision are terrible



  • The magic 40 should be a properly named constant



  • The one line of comment for collision is pretty useless, moar comments are needed



-
This :

if (r1  r2) return false;
return true;`


could be

return !(r1  r2)


All in all, I think this code requires some polishing, but the basics are okay.

Code Snippets

if (r1 < x2 || x1 > r2) return false;
return true;`
return !(r1 < x2 || x1 > r2)

Context

StackExchange Code Review Q#46915, answer score: 2

Revisions (0)

No revisions yet.