Class enumeration setter, convention javascript

Question

Hi I have the following class:

const MyClass = function MyClass() {
  this.state = null;
}

MyClass.prototype.set = function(key, value) {
    this[key] = value;
}

And the following enum in an other module:

const enumeration = {
    ACTIVE: 'active',
    PENDING: 'pending',
    DONE: 'done'
}

I would like to know what are the best practices / conventions to handle setter for enumerations properties (state here)

I could do something like this:

let myClass = new MyClass();
myClass.set('state', enumeration.PENDING);

but its a bit verbose, and I need to know about the myClass module and the enum module.
An other idea is to do that:

const MyClass function MyClass() {
    this.state = null;
}

MyClass.prototype.setPending = function() {
    this.state = enumeration.PENDING;
}

and allow to call myClass.setPending() But I don't know about the right naming of this kind of setters.

Answer 1

First I would argue that this question is not opinion-, but needs-based. If you say that it is preference-based, that means you simply have not encountered the issues which best practices are established to avoid. With that in mind, the first best practice is to keep it as simple as possible. How your API evolves depends on your application's specific needs and your ability to spot which ones are most appropriate.

Below is an example "evolution of code" of an upload object for a web FTP client. Note that there are a lot of different possible paths. This example's only aim is to demonstrate that best practices are not just a matter of preference.

You start off with this:

function Upload(state) {
  this.state = state || 'pending';
  this.elem = document.createElement('div');
}
new Upload().state = 'active';

Then you realize you need a progressbar view to update when the state is set, so you make a specialized setter:

Upload.prototype.setState = function(value) {
  this.state = value;
  this.elem.className = 'state-' + this.state;
};

However later on you decide that you need to display a notification, but only when the state is set to 'done'. Or perhaps you find yourself in need of a shorthand method because you have to type upload.setState('done') a lot and find typing upload.setStateDone() easier (cf. jQuery's $.get vs $.ajax).

Upload.prototype.setStateDone = function() {
  this.setState('done');
  // code to execute only when the state changes to 'done'
};

Note that you can prefer consistent API over convenience (e.g. I do prefer $.get over $.ajax because there is significant overhead in using the latter, but I prefer to use jQuery's $(elem).on('click',.. over $.click because all it does is prefill the 'click' string).

There's still a problem with the setter: if you set an invalid state, e.g. 'whatever', it will attach the class 'state-whatever', so you update setState:

Upload.prototype.setState = function(value) {
  if (['active', 'pending', 'done'].indexOf(value) > -1)
    this.state = value;
  this.elem.className = 'state-' + this.state;
};

Problem is, your states are hard-coded in a method and you would like to re-use them in other components, so you define a states map and adjust setState. In order to avoid a hard dependency, you use dependency injection instead of referencing the global states map inside the constructor:

var states = {
  ACTIVE: 'active',
  PENDING: 'pending',
  DONE: 'done'
};

function Upload(state, states) {
  this.states = states;
  this.state = state || this.states.PENDING;
  this.elem = document.createElement('div');
}

Upload.prototype.setState = function(value) {
  var states = Object.keys(this.states).map(function(key) {
     return key.toLowerCase();
  });
  if (states.indexOf(value) > -1) {
    this.state = value;
  this.elem.className = 'state-' + this.state;
};

On a sidenote, a setter in its most basic form is just a function that sets a property to a value on an object. How specific you make it, depends on how many parameters you hardcode into the setter.

function createSetter(context, prop, val) {
  switch (arguments.length) {
     case 0:
       return function(p, v) { this[p] = v; };
     case 1:
       return function(p, v) { this[p] = v; }.bind(context);
     case 2:
       return function(v) { this[prop] = v; }.bind(context);
     case 3:
       return function() { this[prop] = val; }.bind(context);
  }
}

var genericSetter = createSetter(); // any prop/val, implicit this context,
    boundSetter = createSetter({}); // any prop/val, fixed context
    propSetter = createSetter({'hello': 'world'}, 'hello') // fixed prop, any val, defined context
    propValSetter = createSetter({'hello': 'world'}, 'hello', 'dude') // fixed prop, fixed val, fixed context

Answer 2

I would go with the second option since:

1. Avoids using hardcored strings ('state') which I think is always good.
2. Makes calls shorter to write and read.
3. If you decided to change the behaviour, would you rather change the setPending function or every set call?

I think the third point is the most important one, try to make your code as mantainable as possible.