1

This is an extension to my previous post: Using useState React hooks and calling other functions

In my code, I have several fields like firstName, lastName, age, gender, city, state, country, etc. So the approach which I am following now is lithe this:

function MyApp() {
    const [firstName, setFirstName] = React.useState("");
    const [lastName, setLastName] = React.useState("");
    const [city, setCity] = React.useState("");
    const [gender, setGender] = React.useState("");
    const [age, setAge] = React.useState(10);
    const [completion, setCompletion] = useState(0);
    
    calculateProgress = (firstName, lastName, city, gender, age) => {
        let total = 0;
        if(firstName.length >0 ) total += 50;
        if(lastName.length >0 ) total += 50;
        if(city.length >0 ) total += 50;
        if(gender.length >0 ) total += 50;
        if(age >0 ) total += 50;
        
        setCompletion(total);
    }

    return {
        <div>
            <input id="firstName" onChange={(e) => setFirstName(e.target.value); calculateProgress(e.target.value, lastName, city, gender, age);}
            <input id="lastName" onChange={(e) => setLastName(e.target.value); calculateProgress(firstName, e.target.value, city, gender, age);}
            <input id="city" onChange={(e) => setCity(e.target.value); calculateProgress(firstName, lastName, e.target.value, gender, age);}
            <input id="gender" onChange={(e) => setGender(e.target.value); calculateProgress(firstName, lastName, city, e.target.value, age);}
            <input id="age" onChange={(e) => setAge(e.target.value); calculateProgress(firstName, lastName, city, gender, e.target.value);}
        </div>
    }
}

I want to optimize this code, I have declared so many variables like firstName, lastName, gender and also I have to pass multiple values for each calculateProgress function call. What is the correct way to avoid this.

learner
  • 6,062
  • 14
  • 79
  • 139
  • Pass a single object with properties instead of a bunch of individual arguments where order matters and it's easy to screw up. – Drew Reese Jul 06 '21 at 22:04

2 Answers2

2

Pass a single object with properties instead of a bunch of individual arguments where order matters and it's easy to screw up.

It would better to call calculateProgress in an useEffect so it's referencing the updated state, then you won't need to pass any arguments to it at all.

function MyApp() {
  const [firstName, setFirstName] = React.useState("");
  const [lastName, setLastName] = React.useState("");
  const [city, setCity] = React.useState("");
  const [gender, setGender] = React.useState("");
  const [age, setAge] = React.useState(10);
  const [completion, setCompletion] = useState(0);
    
  calculateProgress = () => {
    let total = 0;
    if (firstName.length) total += 50;
    if (lastName.length) total += 50;
    if (city.length) total += 50;
    if (gender.length) total += 50;
    if (age > 0) total += 50;
        
    setCompletion(total);
  }

  useEffect(() => {
    calculateProgress();
  }, [firstName, lastName, city, gender, age]); 

  return {
    <div>
      <input id="firstName" onChange={(e) => setFirstName(e.target.value)} />
      <input id="lastName" onChange={(e) => setLastName(e.target.value)} />
      <input id="city" onChange={(e) => setCity(e.target.value)} />
      <input id="gender" onChange={(e) => setGender(e.target.value)} />
      <input id="age" onChange={(e) => setAge(e.target.value)} />
    </div>
  }
}
Drew Reese
  • 165,259
  • 14
  • 153
  • 181
  • 1
    Agreed, this is what I would do. The example code is missing `()` after `calculateProgress`. Also, I wonder whether the function should be inlined into the effect? – edemaine Jul 06 '21 at 22:09
  • 1
    @edemaine I was cleaning up syntax, but good catch. Regarding inlining, I believe it is more clear/correct to call `calculateProgress` from hook's callback function for 2 reasons, (1) if `calculateProgress` returns a value it would likely incorrectly be interpreted as an `useEffect` hook cleanup function, and (2) if later a cleanup function *did* need to be added it's a simple insertion and the line with `calculateProgress` remains untouched in the git blame. – Drew Reese Jul 06 '21 at 22:12
1

This isn't necessarily the correct way, but one way to avoid creating so many variables is to use a single object in state instead. Map over an array of properties when creating the inputs.

const properties = ['firstName', 'lastName', 'city', 'gender', 'age'];
function MyApp() {

    const [formValues, setFormValues] = React.useState({
        firstName: '',
        lastName: '',
        city: '',
        gender: '',
        age: 10,
    });
    const [completion, setCompletion] = useState(0);
    calculateProgress = ({ firstName, lastName, city, gender, age }) => {
        let total = 0;
        if (firstName.length > 0) total += 50;
        if (lastName.length > 0) total += 50;
        if (city.length > 0) total += 50;
        if (gender.length > 0) total += 50;
        if (age > 0) total += 50;

        setCompletion(total);
    }
    const changeHandler = (e) => {
        const newValues = { ...formValues, [e.target.id]: e.target.value };
        setFormValues(newValues);
        calculateProgress(newValues);
    };
    return (
        <div>
            {
                properties.map(prop => (
                    <input id={prop} onChange={changeHandler} />
                ))
            }
        </div>
    )
}
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • Thanks this looks simple, what if Gender is a dropdown with limited values, also I have other fields which are select boxes and radio buttons. Is there a simple way for them also to use with properties.map as you suggested here – learner Jul 06 '21 at 22:15
  • I don't think so. If they don't all have the same JSX, those that aren't a simple `` will have to be written out. If the inputs aren't contiguous, you'll probably have to give up on the `properties.map` too. – CertainPerformance Jul 06 '21 at 22:17