JQuery/Javascript - how to minimize duplicated code?

Question

If I wanted to use the following code on multiple DIV#ID, how do I do so without duplicating code

var scrollElem = $('#div1');
scrollElem.scroll(function() {
 /* find the closest (hlisting) home listing to the middle of the scrollwindow */ 
    var scrollElemPos = scrollElem.offset();
    var newCenter = $(document.elementFromPoint(
        scrollElemPos.left + scrollElem.width()  / 2,
        scrollElemPos.top  + scrollElem.height() / 2)
    ).closest('.hlisting');
    if(newCenter.is(".HighlightRow")) return;
    $('.HighlightRow').removeClass("HighlightRow");
    newCenter.addClass('HighlightRow');
});

What I want to do is perform this not only on div1, but also on div2, div3, div4.

But as you note, scrollElem is a global variable so I can't just stick all of this code in 1 function.

Meaning, to get this to work - I would have to do:

// DIV 2 ---------------------------
var scrollElem2 = $('#div2');
scrollElem.scroll(function() {
 /* find the closest (hlisting) home listing to the middle of the scrollwindow */ 
    var scrollElemPos = scrollElem2.offset();
    var newCenter = $(document.elementFromPoint(
        scrollElemPos.left + scrollElem2.width()  / 2,
        scrollElemPos.top  + scrollElem2.height() / 2)
    ).closest('.hlisting');
    if(newCenter.is(".HighlightRow")) return;
    $('.HighlightRow').removeClass("HighlightRow");
    newCenter.addClass('HighlightRow');
});

// DIV 3 ---------------------------
var scrollElem3 = $('#div3');
scrollElem3.scroll(function() {
 /* find the closest (hlisting) home listing to the middle of the scrollwindow */ 
    var scrollElemPos = scrollElem3.offset();
    var newCenter = $(document.elementFromPoint(
        scrollElemPos.left + scrollElem3.width()  / 2,
        scrollElemPos.top  + scrollElem3.height() / 2)
    ).closest('.hlisting');
    if(newCenter.is(".HighlightRow")) return;
    $('.HighlightRow').removeClass("HighlightRow");
    newCenter.addClass('HighlightRow');
});

That's copying and pasting a lot of duplicate code.

Question: there must be a better way to do this. Any ideas on how to accomplish the same functionality but minimize the duplication of code.

Gabriele Petrioli · Accepted Answer · 2010-01-28 19:10:44Z

4

use multiple selector when you define the scrollElem

var scrollElem = $('#div1, #div2, ...');

and inside the function, wherever you want to use the current scrollElem use $(this)

var scrollElemPos = $(this).offset();

etc..

answered Jan 28, 2010 at 19:10

Gabriele Petrioli

197k35 gold badges275 silver badges331 bronze badges

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

1 Comment

Gabriele Petrioli Over a year ago

@ALlen, well.. instead of using $(this) multiple times you could create a variable like var $this = $(this) and use that ... that would be optimal i think..

munch · Accepted Answer · 2010-01-28 19:10:25Z

2

Put it into a function.

function myFunc(elem){
    var scrollElemPos = elem.offset();
    var newCenter = $(document.elementFromPoint(
        scrollElemPos.left + elem.width()  / 2,
        scrollElemPos.top  + elem.height() / 2)
    ).closest('.hlisting');
    if(newCenter.is(".HighlightRow")) return;
    $('.HighlightRow').removeClass("HighlightRow");
    newCenter.addClass('HighlightRow');
}

var scrollElem = $('#div1');
scrollElem.scroll(function() {
  myFunc($(this));
});

answered Jan 28, 2010 at 19:10

munch

6,3214 gold badges26 silver badges27 bronze badges

6 Comments

Ty W Over a year ago

have to change the reference to scrollElem3 as it would need to be different for each object. $(this) seems like it'd work.

Damien Wilson Over a year ago

Beat me to the punch. There's no reason the code given by the OP can't be put into a function. If anything, it's best to avoid global variables entirely.

Allen Over a year ago

What about scrollElem3 in the myFunc()?

munch Over a year ago

@ty & @Allen, missed it...changed to 'elem'

Allen Over a year ago

Couldn't I also eliminate all together the scrollElem by doing: $('#div1').scroll(function(){myFunc($(this));});

|

2 revs · Accepted Answer · 2017-05-23 10:24:37Z

1

I think a combination of the answers provided by Gaby and munch would be optimal:

Use a multi-selector and $(this),

$('#div1, #div2, ...').scroll(myFunc);

In combination with a predefined function:

function myFunc() {
    var scrollElemPos = $(this).offset();
    // etc...
}

Now existing functionality works as designed, and you can also call myFunc manually with call and apply.

// call myFunc with .call or .apply to set context
myFunc.call(someElement); // inside myFunc "this" will point to someElement

edited May 23, 2017 at 10:24

community wiki

2 revs
Joel

Comments

Ty W · Accepted Answer · 2010-01-28 19:10:38Z

-1

couldn't you change the anonymous function to a named function and pass the same function to whatever.scroll?

the only thing you'd have to change would be the reference to scrollElemX. use $(this) instead.

answered Jan 28, 2010 at 19:10

Ty W

6,8245 gold badges30 silver badges37 bronze badges

Collectives™ on Stack Overflow

JQuery/Javascript - how to minimize duplicated code?

4 Answers 4

1 Comment

6 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

6 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related