How should I define my constructor (JS)

Question

Currently my constructor has been defined like this and working :

function Carousel(parent, currentItem, count) {
    var element = parent;
    var count = count;
    var arrPrev = document.createTextNode('‹');
    var arrNext = document.createTextNode('›');
    var cPrevClass = 'cPrevClass';
    var cNextClass = 'cNextClass';
    var crslClass = 'js-Carousel';
    var current = currentItem;

    showArrows();

    /**
    * Create the navigation arrows (prev/next) and attach to carousel.
    */
    function showArrows() {
       // stuff here
       // Event listener (important for my question)
        buttonPrev.addEventListener('click', showPrev);
        buttonNext.addEventListener('click', showNext);
    }
}

Because at the time I somehow forgot that I'm supposed to define my class constructor using the this keyword (I guess), I now have rewritten it like this :

function Carousel(parent, currentItem, count) {
    this.element = parent;
    this.count = count;
    this.current = currentItem;
    this.showArrows = showArrows;
    this.showArrows();
}

/**
 * Create the navigation arrows (prev/next) and attach to carousel.
 */
function showArrows() {
    // stuff here...
    var modal = document.getElementById("netreviews_media_modal");

    // then adding event listener
    buttonPrev.addEventListener('click', showPrev);
    buttonNext.addEventListener('click', showNext);

    modal.appendChild(buttonPrev);
    modal.appendChild(buttonNext);
}

/**
 * Go back
 */
function showPrev() {
    if (Carousel.current == 0) {
        Carousel.current = Carousel.count - 1;
    }
    else {
        Carousel.current = Carousel.current - 1;
    }
    updateModal(element.querySelectorAll('.' + crslClass + ' > ul li > a')[Carousel.current]);
}

Now when the updateModal() function is running (see at the end of file), I have that error that tells me "Cannot read property 'querySelectorAll' of undefined" Of course, element isn't defined anymore and just says 'undefined'.

I don't know how to make the element point to my class property. Also, Carousel.current isn't working either. And I dont know if Im doing good refactoring that. I haven't put all code (//stuff here) but I think its enough... (FYI I do instantiate my Carousel function later in the code) Any help appreciated !

Indeed. But you aren't helping. How can I access all of these properties from Carousel in later functions? — DevMoutarde
– DevMoutarde, Commented Mar 6, 2018 at 10:14
Well, you have to instantiate the object if you want to use it — Mansuro
– Mansuro, Commented Mar 6, 2018 at 10:17
I did actually, in a later function, but still can't access properties of Carousel in my event listener function... — DevMoutarde
– DevMoutarde, Commented Mar 6, 2018 at 10:18

Erik Terwan · Accepted Answer · 2018-03-06 12:00:43Z

1

There are a couple of ways to do this, the best way being the es6 or ECMAScript2015 way. Just create a class like so:

class Foo {
  constructor(bar) {
    this.bar = bar
  }

  baz() {
    console.log(this.bar) // prints Hello world
  }
}

const foo = new Foo('Hello world') // your instance
foo.baz()

Another way would be the 'old' way, by creating a function, and you're pretty close. There is however one issue;

function Foo(bar) {
  this.bar = bar;

  this.baz = function() {
    console.log(this.bar) // this would point to the anonymous function scope, not the class scope, so this.bar would be undefined
  }
}

const foo = new Foo('Hello world') // your instance
foo.baz() // throws an error

As noted in the comments, this is not the class or Foo scope, but rather the scope of the anonymous function. How i usually fix that, is by using self executing anonymous functions. Now the this scope is saved in self, you can use it like this;

function Foo(bar) {
  this.bar = bar;

  this.baz = (function(self) {
    return function() {
      console.log(self.bar); // prints Hello world
    }
  })(this);
}

const foo = new Foo('Hello world') // your instance
foo.baz() // all good

As @Kulvar pointed out, it's also possible (and maybe more performant) to store the this in a constant / variable. I personally do not really like this approach, it feels a bit hacky;

function Foo(bar) {
  this.bar = bar;

  var self = this;

  this.baz = function() {
    console.log(self.bar) // note the use of `self` instead of `this`
  }
}

const foo = new Foo('Hello world') // your instance
foo.baz() // all good

edited Mar 6, 2018 at 12:00

answered Mar 6, 2018 at 10:19

Erik Terwan

2,79020 silver badges28 bronze badges

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

10 Comments

Kulvar Over a year ago

You should use something like const anchor = this; instead of using an IIFE. Performance are better because you don't create a new function with its scope

DevMoutarde Over a year ago

Thanks to you both, trying to fully understand it and try it, I ll hit you up when I have :)

DevMoutarde Over a year ago

Im trying to make it to work, for now I cant seem to understand why you put baz with quotes... never seen that before ahah I may be msising something. Also I haven't succeed calling my showArrows() function.... I ll update my code to show you what I've tried. Im trying the old way for now

Erik Terwan Over a year ago

Ah 'baz' is just a string i'm passing. Let me update my answer for clarity.

DevMoutarde Over a year ago

Ive got it to work, finally ! Thanks to your answer that made me realize the issue was with the scope. I put your answer as accepted since it really helped at the end, but all these answers has been really helpful

|

Andrew Bone · Accepted Answer · 2018-03-06 10:24:20Z

1

To convert this into an actual class it would have to look something like this.

You'll see at the bottom I have a variable that is an instance of the class you can then use call your functions on this variable carousel.showPrev() for instance can be called from outside the class.

class carouselClass {
  constructor(parent, currentItem, count) {
    this.element = parent;
    this.count = count;
    this.arrPrev = document.createTextNode('‹');
    this.arrNext = document.createTextNode('›');
    this.cPrevClass = 'cPrevClass';
    this.cNextClass = 'cNextClass';
    this.crslClass = 'js-Carousel';
    this.current = currentItem;

    this.showArrows();
  }

  // Create the navigation arrows (prev/next) and attach to carousel.
  showArrows() {
    // stuff here...
    let modal = document.getElementById("netreviews_media_modal");

    // then adding event listener
    buttonPrev.addEventListener('click', this.showPrev);
    buttonNext.addEventListener('click', this.showNext);

    modal.appendChild(buttonPrev);
    modal.appendChild(buttonNext);
  }

  // Go back
  showPrev() {
    if (this.current == 0) {
      this.current = this.count - 1;
    } else {
      this.current--;
    }
    updateModal(this.element.querySelectorAll('.' + this.crslClass + ' > ul li > a')[this.current]);
  }
}

let carousel = new carouselClass('parent', 'currentItem', 'count');

Here is some further reading

I hope you find this helpful.

answered Mar 6, 2018 at 10:24

Andrew Bone

7,3112 gold badges21 silver badges35 bronze badges

16 Comments

DevMoutarde Over a year ago

Thanks, about to read that, trying to understand it all ahah, will get back to you asap

DevMoutarde Over a year ago

This actually seems to be the easiest way to achieve what I want, I'm gonna try all solutions, haven't been able to succeed yet (cuz Im a noob still ahah)

DevMoutarde Over a year ago

So I've tried your solution, as well as the others, but I still can't access my 'element' in the showPrev() function, I don't understand why.....

Andrew Bone Over a year ago

And you're using this.? Can you use jsfiddle.com to make a demo of your app not working, paste the link in here and I'll have a look

DevMoutarde Over a year ago

Yes I am, ok Im gonna go fiddle :)

|

Kulvar · Accepted Answer · 2018-03-06 10:45:12Z

1

You're using Carousel in the showPrev function, but Carousel isn't an object, it's a class.

function MyClass(arg1)
{
    this.myProperty = arg1;
}

// If you want your class to inherit
MyClass.prototype = Object.create(MyParentClass.prototype);

MyClass.prototype.myMethod = function ()
{
    // You can use this inside a method
    console.log(this.myProperty);
};

MyClass.MyStaticProperty = 42;
MyClass.MyStaticMethod = function ()
{
    // You can't use this here
};

const myObject = new MyClass("Hello, World");
myObject.myMethod();

Replace "Carousel" by "this" and attach the method to the class prototype. Then instanciate your class into an object.

edited Mar 6, 2018 at 10:45

answered Mar 6, 2018 at 10:18

Kulvar

1,1799 silver badges22 bronze badges

1 Comment

DevMoutarde Over a year ago

Thanks for the useful info, I've managed to get it to work thanks to all these answers

Collectives™ on Stack Overflow

How should I define my constructor (JS)

3 Answers 3

10 Comments

16 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

10 Comments

16 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related