I'm trying to create multiple instances of a slider on a page. Each slider should know which slide it’s currently viewing. It seems that when I update the slide property, I change it for the class, and not the instance. This suggests to me that I'm not instantiating properly in my public init() function. Where am I going wrong?

var MySlider = (function() {

    'use strict';

    var animating = 0,
           slides = 0, // total slides
           slider = null,
            slide = 0, // current slide
             left = 0;

    function slideNext(e) {
        if ((slide === slides - 1) || animating) return;

        var slider = e.target.parentNode.children[0],
                 x = parseFloat(slider.style.left);

        animate(slider, "left", "px", x, x - 960, 800);
        slide++;
    }

    return {
        init: function() {
            var sliders = document.querySelectorAll('.my-slider'),
                      l = sliders.length;

            for (var i = 0; i < l; i++) {
                sliders[i] = MySlider; // I don't think this is correct.

                slider = sliders[i];

                buildSlider(slider);

            }
        }
    }

})();

I'm trying to create multiple instances of a slider on a page. Each slider should know which slide it’s currently viewing. It seems that when I update the slide property, I change it for the class, and not the instance. This suggests to me that I'm not instantiating properly in my public init() function. Where am I going wrong?

var MySlider = (function() {

    'use strict';

    var animating = 0,
           slides = 0, // total slides
           slider = null,
            slide = 0, // current slide
             left = 0;

    function slideNext(e) {
        if ((slide === slides - 1) || animating) return;

        var slider = e.target.parentNode.children[0],
                 x = parseFloat(slider.style.left);

        animate(slider, "left", "px", x, x - 960, 800);
        slide++;
    }

    return {
        init: function() {
            var sliders = document.querySelectorAll('.my-slider'),
                      l = sliders.length;

            for (var i = 0; i < l; i++) {
                sliders[i] = MySlider; // I don't think this is correct.

                slider = sliders[i];

                buildSlider(slider);

            }
        }
    }

})();

• javascript

Share Improve this question Follow asked Apr 24, 2013 at 14:17 Jezen ThomasJezen Thomas 13.8k77 gold badges5555 silver badges9595 bronze badges 3

• I don't get the purpose of this way of creating things except adding useless plexity and sources of mistakes. Can someone enlighten me ? – Virus721 Commented Apr 24, 2013 at 14:26
• 1 your "I don't think this is correct" line is well assessed. you are attaching the same object to every slider. My inclination would be to invert this and make MySlider a constructor that takes a parameter (a single slider element) on which to operate. Make a new MySlider for each slider element you have. – underrun Commented Apr 24, 2013 at 14:28
• @underrun Could you show me how? The idea was that when someone implements this module, they could just call MySlider.init(); once their DOM has loaded, and my function would loop over matched elements on the page and create an instance for each. I wouldn’t want them to have to manually create instances. – Jezen Thomas Commented Apr 24, 2013 at 14:31

Add a ment  | 

2 Answers 2

Sorted by: Reset to default 5

Based on your ment, I think this is more like what you want:

MySlider = (function () {
    Slider = function (e) {
        this.e = e;
        // other per element/per slider specific stuff
    }

    ...

    var sliders; // define this out here so we know its local to the module not init

    return {
        init: function () {

            sliders = document.querySelectorAll('.my-slider');
            var l = sliders.length;

            for (var i = 0; i < l; i++) {
                sliders[i] = new Slider(sliders[i]); //except I'd use a different array

                slider = sliders[i];

                buildSlider(slider);
        }
    }
})();

This way, you are associating each element with it's own element specific data but you have a containing module within which you can operate on your collection of modules.

It seems that when I update the slide property, I change it for the class, and not the instance.

You are correct. That code is run only when the MySlider class is defined. If you want an instance variable, you need to declare it inside the returned object, ie, part of your return block:

 var MySlider = (function(param) {
    return {
       slider: param, 

       init: function() {
            var sliders = document.querySelectorAll('.my-slider'),
                      l = sliders.length;

            for (var i = 0; i < l; i++) {
                sliders[i] = MySlider; // I don't think this is correct.

                slider = sliders[i];

                buildSlider(slider);

            }
        }
});