2

I'm rewriting some old Chrome extension code while simultaneously trying to learn new ES6 tricks, and I'm running into some design questions.

My goal is to provide a value storage (which is backed by the asynchronous chrome.storage for persistence, but that's outside the scope of the question). What I want is to associate some validation with the values. So, my Storage is a collection of Values, each associated with a validation function.

In my old version, I would just pass a validation function when I instantiate a value, something like this (simplified):

Storage["key1"] = new Value({
  validator: ValidatorIsInteger, defaultValue: 0, /* ... */
});
Storage["key2"] = new Value({
  validator: ValidatorEnum(["a", "b", "c"]), defaultValue: "a", /* ... */
});

However, I'm trying to rewrite this with Value being a class that can be extended with specific validators, which seemed to be a good idea at the time. Again, simplified:

class Value {
  constructor(key, defaultValue) {
    this.key = key;
    this.defaultValue = defaultValue;
    this.set(defaultValue);
  }

  set(newValue) {
    var validationResult = this.validate(newValue);
    if (validationResult.pass) {
      this.value = newValue;
      return newValue;
    } else {
      throw new RangeError(
        `Value ${newValue} for ${this.key} failed validation: ${validationResult.message}`
      );
    }
  }

  get() { return this.value; }

  // Overload in children
  validate(value) {
    return {pass: true};
  }
}

class IntegerValue extends Value {
  validate(value) {
    if (Number.isInteger(value)) {
      return {pass: true};
    } else {
      return {pass: false, message: "Value must be an integer"};
    }
  }
}

So far so good. However, I run into problems when trying to make a parametrized child class:

class EnumValue extends Value {
  constructor(key, defaultValue, possibleValues) {
    this.possibleValues = possibleValues; // NUH-UH, can't set that before super()
    super(key, defaultValue);
  }

  // Will be called from parent constructor
  validate(value) {
    if (this.possibleValues.includes(value)) {
      return {pass: true};
    } else {
      return {pass: false, message: `Value must be in [${this.possibleValues}]`};
    }
  }
}

The problem is in "setting up" the parametrized validator before .set(defaultValue) is called. I see several ways out of this, all of which seems lacking:

  • Resign, and not use the class-extension-based approach - I want to see if it can be fixed first.
  • Always trust the default value as a workaround to calling .set(defaultValue) - bad, because I don't want accidentally inconsistent data.
  • Make .set() asynchronous, giving the constructor a chance to finish before validation is performed - while the persistence backend is asynchronous, the purpose of Storage is, among other things, to provide a synchronous "cache".

Am I failing to see some obvious fix to this approach? If not, and this is simply a wrong tool for the job, how should I re-organize this?

Xan
  • 74,770
  • 16
  • 179
  • 206

1 Answers1

3

This is the classic problem with calling overrideable methods (validate, via set) from constructors; discussed here (different language, same problem).

Your specific example lends itself to a couple of workarounds, but the general issue remains.

To solve the general problem, I'd set value directly, not via set, and use unit tests to ensure that I didn't create validators that have an invalid default value. That is, after all, a coding error, not a runtime error.

But if you want to keep calling set, a couple of options:

  • You could have the concept of a Value that has no default value, which might be useful in general anyway. That would solve the problem by letting you have a Value constructor that doesn't expect to receive a default value. You could give a Value a default value after construction via setDefaultValue or similar; that method would validate, but that's fine, because it would be called post-construction in the subclass.

  • You could give Value a "validating" and "non-validating" state, and have the constructor accept a flag for which state it should start out in. Subclasses would use false if they had special validation behavior, ensure that all their ducks are in a row, and then set the validation state (which would validate).

Community
  • 1
  • 1
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • Thank you for the "classic problem" link, I have little OOP experience so explanation of standard gotchas is very helpful. – Xan Jun 12 '16 at 17:01