3
\$\begingroup\$

I want to make this code better , its works now, but i think it can be better , i have a hook and one property is onChange that get value and can setState and return that value for value of input , in the middle of code , i should check two regex and if is not valid skip the setState and return that .

    onChange: (value) => {
      const validRegex = /^[\u0600-\u06EF\s]+$/;
      if (!validRegex.test(value)) {
        const regex1 = /[a-zA-Z0-9]/g;
        const regex2 = /[\u06F0-\u06F9]+$/;
        return value.replace(regex1, '').replace(regex2, '');
      }

      setData((prev) => ({
        ...prev,
        name: value,
      }));
      return value;
    },
<input value={data.name} onChange={(e) => onChange(e.target.value)} />

I replace , because i dont want miss text that user wrote. ,

example: value : This is name 1234 ===> result : This is name

\$\endgroup\$
6
  • 2
    \$\begingroup\$ I've given rewriting your title a shot to give you an idea of what we're looking for, feel free to improve it along those lines. \$\endgroup\$
    – Mast
    Jan 6, 2022 at 18:32
  • 2
    \$\begingroup\$ This code removes non-Unicode characters from a string, right? If so, perhaps that can be incorporated into the title \$\endgroup\$ Jan 6, 2022 at 18:54
  • \$\begingroup\$ yeah @peterSO, I want to deprive user just type arabic character , but because i want to to have previous character in the input , i should find that false character and replace with empty , otherwise in the middle string when user entered wrong character , the whole of string converted to empty suddenly \$\endgroup\$ Jan 7, 2022 at 19:10
  • \$\begingroup\$ @tmohammad78 Does your input field look like <input value={data.name} onChange={(e) => onChange(e.target.value)} />? If not, could you please add it to the question? \$\endgroup\$ Jan 10, 2022 at 17:49
  • \$\begingroup\$ @NikitaSkrebets, thank you for your attention \$\endgroup\$ Jan 10, 2022 at 18:58

1 Answer 1

3
\$\begingroup\$

Firstly, regular expression constants are better be placed outside of component function. Otherwise they get recreated each change event.

Second, is there a purpose of changing value? If we enter the if statement, we will not change the state, so it will affect nothing. And we don't need to return value; in the end, too.

Third, in current configuration there is no need to have regex2, because if we get a string \u0600\u06F0, which is a letter and a number (if I understand correctly), we will remove the number later. If it's \u06F0\u0600, a reversed one, we won't remove it. Though anyway, in the end it doesn't matter, because operations inside the if statement won't change the state.

Fourth, we check for /^[\u0600-\u06EF\s]+$/. Because of the + in the end we can't have an empty string. I don't know if it's by design, but if we have only 1 letter, we can't delete it, we can only select it and type another letter to replace it, which may not be convenient.

In the end, the version of code that I see fitting the task is:

  // arabic letters and signs without numbers
  const validRegex = /^[\u0600-\u06EF\s]+$/;

  const MyComponent = () => {

    // ...

    onChange: (value) => validRegex.test(value)
      && setData((prev) => ({ ...prev, name: value }));

  // ...

  }

Please correct me if needed

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Great answer. I would have only added that a regex should have a comment with what was intended \$\endgroup\$
    – konijn
    Jan 13, 2022 at 12:29
  • \$\begingroup\$ @konijn, thank you! Good point, added a comment. \$\endgroup\$ Jan 13, 2022 at 14:50

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.