1
\$\begingroup\$

I have a validator class, whose purpose is to validate input data. Initially I wrote the class like this:

export class ValidatorForInputs {

  constructor(inputData) {
    this._data = inputData;
  }

  validate() {      
    let results = {};

    let validateInputDataForMissingKeys = checkInputDataForMissingKeys(this._data);
    if (validateInputDataForMissingKeys) results = { ...results, ...validateInputDataForMissingKeys };

    let validateInputDataForMissingValues = checkInputDataForMissingValues(this._data);
    if (validateInputDataForMissingValues) results = { ...results, validateInputDataForMissingValues };

    let validateInputDataMaximums = checkInputDataMaximums(this._data);
    if (validateInputDataMaximums) results = { ...results, validateInputDataMaximums };

    return results;

  }

}

to be called like this:

  let validatorForInputs = new ValidatorForInputs(inputData);
  let validationForInputsResult = validatorForInputs.validate();
// do something with validationForInputsResult 

But then I wondered, is the constructor really necessary when the data could be passed directly to the validate method like so:

  export class ValidatorForInputs {

    validate(data) {
      let results = {};

      let validateInputDataForMissingKeys = checkInputDataForMissingKeys(data);
      if (validateInputDataForMissingKeys) results = { ...results, ...validateInputDataForMissingKeys };

      let validateInputDataForMissingValues = checkInputDataForMissingValues(data);
      if (validateInputDataForMissingValues) results = { ...results, validateInputDataForMissingValues };

      let validateInputDataMaximums = checkInputDataMaximums(data);
      if (validateInputDataMaximums) results = { ...results, validateInputDataMaximums };

      return results;

    }

  }

and then called like this:

  let validatorForInputs = new ValidatorForInputs();
  let validationForInputsResult = validatorForInputs.validate(inputData);
// do something with validationForInputsResult 

or even called in a one liner like this:

let validationForInputsResult = ( new ValidatorForInputs() ).validate(inputData);
// do something with validationForInputsResult 

With a little bit of testing, I found that the code behaves the same way. I read the what the docs have to say about constructors but did not find any information that answers this question.

So, in a case like this, should a constructor be used or not? If so, why? If not, why not?

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I think a class provides benefits when you need two things:

  • Persistent data associated with an instance, and
  • Methods that do something with the data

If you only need to consolidate data in a single data structure, an object or array is simpler and more suitable.

If you only need to perform an operation on data and produce a result, and then have other not-very-related code work with the result, a plain function is simpler and more suitable.

Your situation looks to fall into the second category. You have data that you want to validate, but all you want to do is validate, and the possible class wouldn't be doing anything else. If all you're doing with a class to set a property, then retrieve that property in a method to process data, and then you never use the instance again, there's not much point to having the class at all - it's just noise. I'd use a standalone function instead.

export const validate = (data) => {
  const results = {};

  const validateInputDataForMissingKeys = checkInputDataForMissingKeys(data);
  if (validateInputDataForMissingKeys) Object.assign(results, validateInputDataForMissingKeys);

  const validateInputDataForMissingValues = checkInputDataForMissingValues(data);
  if (validateInputDataForMissingValues) Object.assign(results, { validateInputDataForMissingValues });

  const validateInputDataMaximums = checkInputDataMaximums(data);
  if (validateInputDataMaximums) Object.assign(results, { validateInputDataMaximums });

  return results;
};

Then call it like:

const validationForInputsResult = validate(inputData);

Also note that you should strongly prefer using const instead of let. let is a warning to other readers of your code (possibly including yourself, in the future) that you may be reassigning the variable in the future. If you're not going to reassign, better to use const, since it requires less cognitive overhead.

The checkInput functions and validateInput variables look a bit repetitive. If possible, it would be nice to change things so that they return an empty object instead of a falsey value, and have checkInputDataForMissingValues and checkInputDataMaximums return objects with validateInputDataForMissingValues and validateInputDataMaximums properties. That way, you can spread them directly into the results object:

const validate = data => ({
  ...checkInputDataForMissingKeys(data),
  ...checkInputDataForMissingValues(data),
  ...checkInputDataMaximums(data),
});

Having 3 separate checkInputData functions plus validate feels a tiny bit off to me. Organization-wise, you could consider putting them all into an object, together with validate if they're not used anywhere else, that way all the related functionality is in a single data structure:

export const validator = {
  checkInputDataForMissingKeys(data) {
    // ...
  },
  checkInputDataForMissingValues(data) {
    // ...
  },
  checkInputDataMaximums(data) {
    // ...
  },
  validate(data) {
    return {
      ...this.checkInputDataForMissingKeys(data),
      ...this.checkInputDataForMissingValues(data),
      ...this.checkInputDataMaximums(data),
    };
  }
};

and

const validationForInputsResult = validator.validate(inputData);
\$\endgroup\$
2
  • \$\begingroup\$ The 3 separate checkInputData functions were written outside of the exported entity (the class in the original code) so that they could not be accessed by anything other than the script where they were written. The intent was to make them "private" functions. What do you think of that approach? \$\endgroup\$
    – knot22
    Commented Apr 5, 2020 at 0:59
  • 1
    \$\begingroup\$ Sure, that makes sense if all checkInput calls have to go through the main validator function. \$\endgroup\$ Commented Apr 5, 2020 at 1:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.