# Reducing code with two identical objects

I have two identical objects on the page, so I get more of the same lines of code. Can anything be made simpler, perhaps with merging?

for(var i=0;i<ticks.length,i<ticks2.length;i++) {

var tick = ticks[i];
var tick2 = ticks2[i];
var tickCenter = tick.offsetWidth / 2
var offset = (i - currentTick) * pixelsPerTick;
var percent = 1-(Math.abs(i-currentTick)/10);

tick2.style.top = ((offset + center - tickCenter)|0) + "px";
tick2.style.opacity = percent;

tick.style.top = ((offset + center - tickCenter)|0) + "px";
tick.style.opacity = percent;
}

-
Extract the redundant code into a function. –  Dagg Jul 4 at 3:53

You can combine the two identical assignments at the end into the same line rather than recalculating the same value again:

for (var i = 0; i < ticks.length && i < ticks2.length; i++) {

var tick = ticks[i];
var tick2 = ticks2[i];
var tickCenter = tick.offsetWidth / 2
var offset = (i - currentTick) * pixelsPerTick;
var percent = 1 - (Math.abs(i - currentTick) / 10);

tick.style.top = tick2.style.top = ((offset + center - tickCenter) | 0) + "px";
tick.style.opacity = tick2.style.opacity = percent;
}


I also fixed your condition in the for loop so that it safely checks both .length values.

You could also avoid some of the intermediate values, but I personally would preserve them the way you have them because of the readability/understandability advantage of having it this way vs. putting them all in one giant formula.

-

The | 0 trick is a cool way to truncate floats to integers, but it's much less readable and relies on a side-effect for the heavy lifting. Although | 0 is appreciably faster, using Math.floor is preferable since the intent is immediately obvious.

Feel free to disregard this if you've identified this as being a performance bottleneck. If your application is not performing slowly, however, then this may be a case of premature optimisation.

-

I would have gone for this -

var updateStyle = function(items, currentTick, pixelPerTick, center){
for(i =0; i < items.length; i++){
var item = items[i];
var tickCenter = item.offsetWidth / 2
var offset = (i - currentTick) * pixelsPerTick;
var percent = 1-(Math.abs(i-currentTick)/10);

item.style.top = ((offset + center - tickCenter)|0) + "px";
item.style.opacity = percent;
}
};



Because it is configurable and the method is reusable for any other similar items that might occur in the future. This code keeps the option open for easily using the same for other items.

-

You can also remove all vars and leave just one like this:

for (var i = 0; i < ticks.length && i < ticks2.length; i++) {

var tick = ticks[i],
tick2 = ticks2[i],
tickCenter = tick.offsetWidth / 2,
offset = (i - currentTick) * pixelsPerTick,
percent = 1 - (Math.abs(i - currentTick) / 10);

tick.style.top = tick2.style.top = ((offset + center - tickCenter) | 0) + "px";
tick.style.opacity = tick2.style.opacity = percent;
}

-
and what exacly is the benefit in doing so? You saved 16 characters.. and lost readability, extensibility and easy changing of the variables you have. I don't think this is a good change.. –  Vogel612 Jul 4 at 8:22