Refactoring repetitive code -js

Question

I have this really long function that sets all the variable positions. Is it better to leave it like this or try to refactor it? If I refactor it the only way I can think of is to do something like:

 function getPosition(name, id){
    var name+"Box" = id.getBoundingClientRect();
    var name+"Top" = name+"Box".top + window.pageYOffset - de.clientTop;
    var name+"Bottom" = name+"Box".bottom + window.pageYOffset - de.clientTop;

    name+"Diff" = name+"Bottom" - name+"Top";
    var pagePadding = winHight - ame+"Diff";
    name+"StartP" = name+"Top" - (name+"Padding"-100);
    }

Is this any better for speed?

Code I am talking about:

function getElementOffset(){ 
    var de = document.documentElement;
    var winHight = window.innerHeight; 

    var pageBox = featurePage.getBoundingClientRect();
    var pageTop = pageBox.top + window.pageYOffset - de.clientTop;
    var pageBottom = pageBox.bottom + window.pageYOffset - de.clientTop;

    pageDiff = pageBottom - pageTop;
    var pagePadding = winHight - pageDiff;
    pageStartP = pageTop - (pagePadding-100);

    var profileBox = featureProfile.getBoundingClientRect();
    var profileTop = profileBox.top + window.pageYOffset - de.clientTop;
    var profileBottom = profileBox.bottom + window.pageYOffset - de.clientTop;

    profileDiff = profileBottom - profileTop;
    var profilePadding = winHight - profileDiff;
    profileStartP = profileTop - (profilePadding);

    var barOneBox = firstBar.getBoundingClientRect();
    var barOneTop = barOneBox.top + window.pageYOffset - de.clientTop;
    var barOneBottom = barOneBox.bottom + window.pageYOffset - de.clientTop;

    barOneDiff = barOneBottom - barOneTop;
    var barOnePadding = winHight - barOneDiff;
    barOneStartP = barOneTop - (barOnePadding -200);
    barOneStart2P = barOneTop - (barOnePadding -barOnePadding +100);

    var barTwoBox = secondBar.getBoundingClientRect();
    var barTwoTop = barTwoBox.top + window.pageYOffset - de.clientTop;
    var barTwoBottom = barTwoBox.bottom + window.pageYOffset - de.clientTop;

    barTwoDiff = barTwoBottom - barTwoTop;
    var barTwoPadding = winHight - barTwoDiff;
    barTwoStartP = barTwoTop - (barTwoPadding -200);
    barTwoStart2P = barTwoTop - (barOnePadding -barOnePadding +100);

    var barThreeBox = thirdBar.getBoundingClientRect();
    var barThreeTop = barThreeBox.top + window.pageYOffset - de.clientTop;
    var barThreeBottom = barThreeBox.bottom + window.pageYOffset - de.clientTop;

    barThreeDiff = barThreeBottom - barThreeTop;
    var barThreePadding = winHight - barThreeDiff;
    barThreeStartP = barThreeTop - (barThreePadding -200);
    barThreeStart2P = barThreeTop - (barOnePadding -barOnePadding +100);

    scrollAnimations();
}

Whole js file:

var featurePage =  document.getElementById('page-feature')
var pageScroll = document.getElementById('page-scroll'); 
var featureProfile =  document.getElementById('profile-feature')
var profileScroll = document.getElementById('profile-scroll'); 
var profileScrollWrapper = document.getElementById('profile-scroll-wrapper');
var firstBar = document.getElementById('first-bar');
var secondBar = document.getElementById('second-bar');
var thirdBar = document.getElementById('third-bar');

var pageStartP, 
    pageDiff,
    profileStartP,
    profileDiff,
    barOneStartP,
    barOneStart2P,
    barOneDiff,
    barTwoStartP,
    barTwoStart2P,
    barTwoDiff,
    barThreeStartP,
    barThreeStart2P,
    barThreeDiff;

function getElementOffset(){ 
    var de = document.documentElement;
    var winHight = window.innerHeight; 

    var pageBox = featurePage.getBoundingClientRect();
    var pageTop = pageBox.top + window.pageYOffset - de.clientTop;
    var pageBottom = pageBox.bottom + window.pageYOffset - de.clientTop;

    pageDiff = pageBottom - pageTop;
    var pagePadding = winHight - pageDiff;
    pageStartP = pageTop - (pagePadding-100);

    var profileBox = featureProfile.getBoundingClientRect();
    var profileTop = profileBox.top + window.pageYOffset - de.clientTop;
    var profileBottom = profileBox.bottom + window.pageYOffset - de.clientTop;

    profileDiff = profileBottom - profileTop;
    var profilePadding = winHight - profileDiff;
    profileStartP = profileTop - (profilePadding);

    var barOneBox = firstBar.getBoundingClientRect();
    var barOneTop = barOneBox.top + window.pageYOffset - de.clientTop;
    var barOneBottom = barOneBox.bottom + window.pageYOffset - de.clientTop;

    barOneDiff = barOneBottom - barOneTop;
    var barOnePadding = winHight - barOneDiff;
    barOneStartP = barOneTop - (barOnePadding -200);
    barOneStart2P = barOneTop - (barOnePadding -barOnePadding +100);

    var barTwoBox = secondBar.getBoundingClientRect();
    var barTwoTop = barTwoBox.top + window.pageYOffset - de.clientTop;
    var barTwoBottom = barTwoBox.bottom + window.pageYOffset - de.clientTop;

    barTwoDiff = barTwoBottom - barTwoTop;
    var barTwoPadding = winHight - barTwoDiff;
    barTwoStartP = barTwoTop - (barTwoPadding -200);
    barTwoStart2P = barTwoTop - (barOnePadding -barOnePadding +100);

    var barThreeBox = thirdBar.getBoundingClientRect();
    var barThreeTop = barThreeBox.top + window.pageYOffset - de.clientTop;
    var barThreeBottom = barThreeBox.bottom + window.pageYOffset - de.clientTop;

    barThreeDiff = barThreeBottom - barThreeTop;
    var barThreePadding = winHight - barThreeDiff;
    barThreeStartP = barThreeTop - (barThreePadding -200);
    barThreeStart2P = barThreeTop - (barOnePadding -barOnePadding +100);

    scrollAnimations();
}

function scrollAnimations(){
  scrollPage();
  scrollProfile();
  scrollBarOne();
  scrollBarTwo();
  scrollBarThree();
}

function scrollPage(){
  var scrollPageHeight = pageScroll.offsetHeight; //full height of scroll image
  var scrollPos = (window.pageYOffset !== undefined) ? window.pageYOffset : (document.documentElement || document.body.parentNode || document.body).scrollTop;
  var s1 = scrollPos - pageStartP;
  var realPos = -s1/pageDiff;

  var lengthLeft = scrollPageHeight - (pageDiff) // + pageScroll.scrollTop 

  if (realPos > 0.09) {
    transY = 0.09 * lengthLeft;
  } else if(realPos < -1) {
    transY = -1 * lengthLeft;
  } else {
    transY = realPos * lengthLeft;
  }

  pageScroll.setAttribute('style', '-webkit-transform:translate3d(0,' + transY + 'px,0); -ms-transform:translate3d(0,' + transY + 'px,0); transform:translate3d(0,' + transY + 'px,0);'); 

}

function scrollProfile(){
  var scrollProfileWidth = profileScroll.offsetWidth; 
  var scrollPos = (window.pageYOffset !== undefined) ? window.pageYOffset : (document.documentElement || document.body.parentNode || document.body).scrollTop;
  var s1 = scrollPos - profileStartP;
  var realPos = -s1/profileDiff;

  var lengthLeft = scrollProfileWidth - (profileDiff)

  if (realPos > 0) {
    transX = 0 * lengthLeft;
  } else if(realPos < -0.604) {
    transX = -0.604 * lengthLeft;
  } else {
    transX = realPos * lengthLeft;
  }
    profileScroll.setAttribute('style', '-webkit-transform:translate3d('+(-transX) + 'px,0,0); -ms-transform:translate3d('+ (-transX) + 'px,0,0); transform:translate3d('+ (-transX) + 'px,0,0);'); 

}

function scrollBarOne() {
  var scrollBarOneWidth = firstBar.offsetWidth;
  var scrollPos = (window.pageYOffset !== undefined) ? window.pageYOffset : (document.documentElement || document.body.parentNode || document.body).scrollTop;
  var s1 = scrollPos - barOneStartP;
  var realPos = s1/barOneDiff;
  var s2 = scrollPos - barOneStart2P;
  var realPos2 = -s2/barOneDiff;
  var lengthLeft = scrollBarOneWidth - (barOneDiff);

  barScrollPosition(realPos, realPos2, lengthLeft, firstBar);
}

function scrollBarTwo() {
  var scrollBarTwoWidth = secondBar.offsetWidth; 
  var scrollPos = (window.pageYOffset !== undefined) ? window.pageYOffset : (document.documentElement || document.body.parentNode || document.body).scrollTop;
  var s1 = scrollPos - barTwoStartP;
  var realPos = s1/barTwoDiff;
  var s2 = scrollPos - barTwoStart2P;
  var realPos2 = -s2/barTwoDiff;
  var lengthLeft = scrollBarTwoWidth - (barTwoDiff) 

  barScrollPosition(realPos, realPos2, lengthLeft, secondBar);

}

function scrollBarThree() {
  var scrollBarThreeWidth = thirdBar.offsetWidth;
  var scrollPos = (window.pageYOffset !== undefined) ? window.pageYOffset : (document.documentElement || document.body.parentNode || document.body).scrollTop;
  var s1 = scrollPos - barThreeStartP;
  var realPos = s1/barThreeDiff;
  var s2 = scrollPos - barThreeStart2P;
  var realPos2 = -s2/barThreeDiff;
  var lengthLeft = scrollBarThreeWidth - (barThreeDiff)

  barScrollPosition(realPos, realPos2, lengthLeft, thirdBar);

}

function barScrollPosition(realPos, realPos2, lengthLeft, targetBar) {
  if (realPos > -9) {
    if (realPos > 1) {
      transX = 1 * lengthLeft;
    } else if(realPos < -0.094) {
      transX = -0.094 * lengthLeft;
    } else {
      transX = realPos * lengthLeft;
    }
  } 

  if (realPos2 < 9) {
    if (realPos2 < -0.183) {
      transX = -0.183 * lengthLeft;
    } else if(realPos2 > 0.51) {
      transX = 0.51 * lengthLeft;
    } else {
      transX = realPos2 * lengthLeft;
    }
  }

  targetBar.setAttribute('style', '-webkit-transform:translate3d('+(-transX) + 'px,0,0); -ms-transform:translate3d('+ (-transX) + 'px,0,0); transform:translate3d('+ (-transX) + 'px,0,0);'); 
}

window.addEventListener('resize', getElementOffset);
document.addEventListener('scroll', getElementOffset);

Whenever doing actions to several different things, I'd say it's always best to turn it into a function (or at the very least, do it recursively). I'm not sure how it impacts speed, but it most certainly makes it cleaner and easier to modify in the future; if you need to change the way you're doing things, you only have to alter the function rather than 10 instances of the same code pasted in a row. — Rein
– Rein, Commented May 1, 2015 at 20:52
You can't concatenate to variable names like that. You should use objects instead of separate variables. — Barmar
– Barmar, Commented May 1, 2015 at 20:53
good to know! thanks Barmar and good point Rein. Im only concerned because this gets called every time the user scrolls, which is a little crazy — ReganPerkins
– ReganPerkins, Commented May 1, 2015 at 20:56
may I ask what you are actually trying to achieve? It looks like this maybe could be done much easier via css. — Jeff
– Jeff, Commented May 1, 2015 at 20:59
Why do you have barOnePadding - barOnePadding in some of you formulas? They just cancel each other out. — Barmar
– Barmar, Commented May 1, 2015 at 21:04

Barmar · Accepted Answer · 2015-05-01 21:03:21Z

2

Use objects instead of separate variables. Then extract the common code from into a function that creates the obect:

function getTopBottom(el, de) {
    var result = {};
    var box = el.getBoundingClientRect();
    result.top = box.top + window.pageYOffset - de.clientTop;
    result.bottom = box.bottom + window.pageYOffset - de.clientTop;
    result.diff = result.bottom - result.top;
    result.padding = window.innerHeight - result.diff;
    return result;
}

Then in your getElementOffset function you can write:

var page = getTopBottom(featurePage, de);

answered May 1, 2015 at 21:03

Barmar

789k57 gold badges554 silver badges669 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

instantaphex · Accepted Answer · 2015-05-01 21:08:31Z

1

First, work on your naming conventions. A function called getElementOffset should return an element offset. Trying to use this code would be really difficult because your naming conventions don't help the reader to understand what is going on.

Second, create reusable objects for your boxes and other elements.

Refactoring doesn't always have to be about speed. Sometimes things can be fast enough. Your code is really hard to read and if you broke up your logic into functions that each had a single responsibility and names that made sense, it would be MUCH easier to read in the future or for another developer that is trying to maintain your code.

answered May 1, 2015 at 21:08

instantaphex

1,0011 gold badge10 silver badges22 bronze badges

Collectives™ on Stack Overflow

Refactoring repetitive code -js

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related