as an exercise, I am trying to find the most efficient way to refactor this if...if statement.

This is the original code:

interface Validatable {
  value: string | number;
  required?: boolean;
  minLength?: number;
  maxLength?: number;
  min?: number;
  max?: number;
}

function validate(validatableInput: Validatable) {
  let isValid = true;
  if (validatableInput.required) {
    isValid = isValid && validatableInput.value.toString().trim().length !== 0;
  }
  if (
    validatableInput.minLength != null &&
    typeof validatableInput.value === 'string'
  ) {
    isValid =
      isValid && validatableInput.value.length >= validatableInput.minLength;
  }
  if (
    validatableInput.maxLength != null &&
    typeof validatableInput.value === 'string'
  ) {
    isValid =
      isValid && validatableInput.value.length <= validatableInput.maxLength;
  }
  if (
    validatableInput.min != null &&
    typeof validatableInput.value === 'number'
  ) {
    isValid = isValid && validatableInput.value >= validatableInput.min;
  }
  if (
    validatableInput.max != null &&
    typeof validatableInput.value === 'number'
  ) {
    isValid = isValid && validatableInput.value <= validatableInput.max;
  }
  return isValid;
}

and this is what I achieved so far:

function validate(validatableInput: Validatable) {
  const { required, minLength, maxLength, min, max } = validatableInput; // This methos extracts the properties from the object
  const validatableInputValue = validatableInput.value;
  let isValid = true;

  if (required) {
    isValid = isValid && validatableInputValue.toString().trim().length !== 0;
  }
  if (minLength != null && typeof validatableInputValue === "string") {
    isValid = isValid && validatableInputValue.length >= minLength;
  }
  if (maxLength != null && typeof validatableInputValue === "string") {
    isValid = isValid && validatableInputValue.length <= maxLength;
  }
  if (min != null && typeof validatableInputValue === "number") {
    isValid = isValid && validatableInputValue >= min;
  }
  if (max != null && typeof validatableInputValue === "number") {
    isValid = isValid && validatableInputValue <= max;
  }
  return isValid;
}

Is there anything else I could do? Like use a switch statement instead, or something else? Thank you!

CodePudding user response：

All of the conditions follow a pattern: if a particular prerequisite is fulfilled, then validate the input value against something. You can exploit this by creating an array of such conditions: each item can have a prerequisite (eg required) and a test (eg value.toString().trim().length !== 0). Then iterate over the array with something like .every to check that each truthy prerequisite has its corresponding condition fulfilled.

function validate(validatableInput: Validatable) {
    const { required, minLength, maxLength, min, max, value } = validatableInput;
    const isStr = typeof value === "string";
    const isNum = typeof value === "number";
    const conditions = [
        [required, value.toString().trim().length !== 0],
        [minLength != null && isStr, value.length >= minLength],
        [maxLength != null && isStr, value.length <= maxLength],
        [min != null && isNum, value >= min],
        [max != null && isNum, value <= max],
    ];
    return conditions.every(([prereq, result]) => result || !prereq);
}

CodePudding user response：

As another approach that keeps the existing structure:

function validate(validatableInput: Validatable) {
  const { value, required, minLength, maxLength, min, max } = validatableInput;

  if (required && value.toString().trim().length === 0) {
    return false;
  }

  if (typeof value !== "string" || typeof value !== "number") {
    return true
  }

  if (typeof value === "string") {
    if (minLength && value.length < minLength) {
      return false;
    }

    if (maxLength && value.length > maxLength) {
      return false;
    }
  }
  
  if (min && value < min) {
    return false;
  }

  if (max && value > max) {
    return false;
  }

  return true;
}

A few things of note:

• Some people have problems with early returns; I don't--it's immediately clear while reading what happens without having to continue reading the function.
• It's not as elegant as the every option, but it allows for easier modification, logging, etc. because it's very explicit and blocked out.
• Since Validatable is a type I'm not sure why this functionality wants to be pulled out of it, particularly since decisions are being made based on type information of a Validatable property.