Validating javascript onChange event

Question

I'm looking to simplify the below code for validating an onChange event. The allowedInput is a regex that is passed (optional) from all the form components.

 const validatedOnChange = (event: ChangeEvent<HTMLInputElement>): void => {
 if (isDisabled) {
 return;
 }
 if (allowedInput) {
 if (allowedInput.test(event.target.value)) {
 onChange({ name, value: event.target.value });
 }
 return;
 }
 onChange({ name, value: event.target.value });
 };

This is a common utility method used for all my React form components. Hence looking to write a readable and consistent code.

Answer 1

Obviously the code does have some redundancy, which violates the Don't Repeat Yourself principle.

This could be simplified by calling onChange() if allowedInput is falsey or if that is not the case then only when the call to the test method on it returns a truethy value:

const validatedOnChange = (event: ChangeEvent<HTMLInputElement>): void => {
 if (isDisabled) {
 return;
 }
 if (!allowedInput || allowedInput.test(event.target.value)) {
 onChange({ name, value: event.target.value });
 }
};

One could opt to take the return early approach, which might be more readable and decrease the indentation on the call to onChange() but would have an extra line:

 if (allowedInput && !allowedInput.test(event.target.value)) {
 return;
 }
 onChange({ name, value: event.target.value });
 

Answer 2

Simplify.

You have 3 conditions checks, isDisabled, "allowedInput", and allowedInput.test, and two effective paths, do nothing, or call onChange. Yet you have 2 returns statements, calls to onChange.

All functions will return you should try to avoid needing to add returns.

You should always avoid repeating the same code. The 2 expressions calling onChange need only be expressed once.

All these things will negatively impact maintenance, and readability.

Destructure

You can also simplify the code using destructure assignment to unpack value from the argument event in the functions arguments negating the need to have the path event.target.value expressed 3 times.

Short circuit

Lastly the if token is just noise and not needed as the call to onChange can be via a short circuit around && The right side is only executed if the left side evaluates to true

This gives you following

const validatedOnChange = ({target: {value}}) => {
 !isDisabled && (!allowedInput || allowedInput.test(value)) && onChange({name, value});
};