Proposal: Move initial config out of the constructor

[PierreAndreis]

Describe the issue

Currently, we are using constructor to set up the config of a class.
Instead, we could use a method of the class. This will keep the code clean, simpler and more dynamic.

Code or steps to reproduce

class Parent {
  constructor(...args) {
    // Do something with it
    console.log(args)
  }
}

class Children extends Parent {
  constructor(...args) {
    super(...args, {
      command: "ping"
    })
  }
}

Expected an actual behaviour

class Children {
  constructor(...args) {
    let config;
    if (typeof this.config === "function") config = this.config();
    if (!config) throw Error("You forgot to set the config using the method config");
    
    // Do something with config or just make it available for the methods
    this._internalConfig = config;
  }
}

class TestChild extends Children {
  config() {
    return {"command": "ping"}
  }
}

[tech6hutch]

I think I like this idea. It's better than the this._patch internal method someone suggested, that reversed it by removing the ...args from the constructor.

[tech6hutch]

Actually, have you...tested this? I'm not sure you can use methods from child classes.

[PierreAndreis]

@tech6hutch I have tested, and yes they are able to
https://jsbin.com/migiqinafe/edit?js,console

[bdistin]

This is a terrible practice (limits the namespace of the class, just to store the same data on the class again later, and that's not even counting any resolved state such as usage. So the data is tripled via instance.config(), instance._internalConfig, and instance.things. As well as there are times you want to add your own state to your class instances, for which that should always be done in the constructor anyway, else you make V8 choke an re-compile your instances every time you add properties.)

Not that you can't do this in your own bot (extend Command or w/e to give yourself this way of setting config)... But this isn't something that will be done in the core framework.

[PierreAndreis]

@bdistin What my proposal was in reality is to eliminate the need of extending args when the user is not going to do anything with it.

config method doesn't need to be a function, it can be just a simple object. This is how React handles their classes. I don't see bad practice in this compared forcing the user to be aware of arguments on constructor that their don't need to.

you make V8 choke an re-compile your instances every time you add properties.

I don't see how you would add properties to the class in runtime (?)

limits the namespace of the class, just to store the same data on the class again later, and that's not even counting any resolved state such as usage. So the data is tripled via instance.config(), instance._internalConfig, and instance.things. As well as there are times you want to add your own state to your class instances, for which that should always be done in the constructor anyway, else you make V8 choke an re-compile your instances every time you add properties.

Again, my demo was an example on what we may do. We can literally force config to be an object and mention it directly.
The bigger picture is that the user doesn't need to be aware of the constructor if he is not responsible for it

[bdistin]

That sentence is meant to be taken as a whole. As in, if you eliminate using constructor, when people want to add properties, they may be tempted to add them in any method, rather than in the constructor which really isn't as optional as it may appear to be.