How to efficiently refactor if...if statement in typescript

Question

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!

What's your definition of "efficient"? If seems like there's two paths, number and string so I'd start there. It also doesn't seem like there's any real need to use the previous isValid in any of them--any invalid data means !isValid. — Dave Newton
– Dave Newton, Commented Apr 7, 2022 at 1:27
Also, if this is a type, it might make sense to wrap up this functionality in that type--my first impression is that there are some missing abstractions. Any time there's flow control based on types there's an opportunity for cleanup, if it makes sense in context. — Dave Newton
– Dave Newton, Commented Apr 7, 2022 at 1:38

CertainPerformance · Accepted Answer · 2022-04-07 01:31:47Z

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);
}

Dave Newton · Accepted Answer · 2022-04-07 01:44:52Z

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.

Collectives™ on Stack Overflow

How to efficiently refactor if...if statement in typescript

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related