Applying DRY principles to JavaScript, help me optimize this code?
While on the search for ways to optimize the quality of my code, I eventually came across the concept of DRY (Don't repeat yourself). I try to follow this as best I can but sometimes I get into positions where I have to write two functions that are practically identical, save for 2 or 3 lines of code and I run out of time while trying to figure out the best way to organize it.
So here's my "question." I've included two functions below that I wrote a couple weeks ago that are basically identical except for 3 lines at the end, as well as one does an animation by addition and the other with subtraction. I would love to get some input from other developers as to how they would optimize the code below OR have examples of unrelated code where you solved a similar problem.
/**
* Go to the previous notification
*
* @private
* @param {object} cl Click event details (ex. {id: 'linkId', ss: '_', index: '1', e: event})
* @memberOf APP.devices
*/
function slideNext (cl) {
var button = $('#' + cl.id + cl.ss + cl.index),
index = cl.index - 1,
slider = devices[index].container.find('.slideContainer'),
// In order to get the value of the 'right' position we must take the (container width - slider width - left position - right-margin)
slidePos = (slider.parent().width() - slider.width()) + (slider.position().left * -1) + (parseFloat(slider.css('margin-right')) * -1);
if (button.hasClass('disabled')) {
return false;
}
slider.find('.active').removeClass('active').prev().addClass('active');
disableButtons(index);
slider.animate({'right': slidePos + notificationOffset}, 200, function () {
determineButtonState(index);
});
updatePositionContext(index);
}
/**
* Advance to the next notification
*
* @private
* @param {object} cl Click event details
* @memberOf APP.devices
*/
function slidePrev (cl) {
var button = $('#' + cl.id + cl.ss + cl.index),
index = cl.index - 1,
slider = devices[index].container.find('.slideContainer');
// In order to get the value of the 'right' position we must take the (container width - slider width - left position - right-margin)
slidePos = (slider.parent(开发者_JAVA百科).width() - slider.width()) + (slider.position().left * -1) + (parseFloat(slider.css('margin-right')) * -1);
if (button.hasClass('disabled')) {
return false;
}
slider.find('.active').removeClass('active').next().addClass('active');
disableButtons(index);
slider.animate({'right': slidePos - notificationOffset}, 200, function () {
determineButtonState(index);
});
updatePositionContext(index);
// Load more notifications once user get's close to the end of the current set of notifications
if (slider.find('.active').nextAll().length == 3) {
getMoreNotifications(index);
}
}
Using a basic flag you can pretty much cut it all out. I'm sure there's a good reason I'm missing for why you haven't done that though, I've never been super-massively big on DRY. Feel free to enlighten me :)
/**
* Move to another notification
*
* @private
* @param {object} cl Click event details (ex. {id: 'linkId', ss: '_', index: '1', e: event})
* @param fw Whether to go forwards or backwards. Defaults to true (forwards)
* @memberOf APP.devices
*/
function slideNext (cl, fw) {
var button = $('#' + cl.id + cl.ss + cl.index),
index = cl.index - 1,
slider = devices[index].container.find('.slideContainer'),
// In order to get the value of the 'right' position we must take the (container width - slider width - left position - right-margin)
slidePos = (slider.parent().width() - slider.width()) + (slider.position().left * -1) + (parseFloat(slider.css('margin-right')) * -1);
var distance = ((fw) ? slidePos + notificationOffset : slidePos - notificationOffset;
if (button.hasClass('disabled')) {
return false;
}
if (fw)
slider.find('.active').removeClass('active').prev().addClass('active');
else
slider.find('.active').removeClass('active').next().addClass('active');
disableButtons(index);
slider.animate({'right': distance}, 200, function () {
determineButtonState(index);
});
updatePositionContext(index);
// Load more notifications once user get's close to the end of the current set of notifications
if (!fw && slider.find('.active').nextAll().length == 3) {
getMoreNotifications(index);
}
}
精彩评论