Cevin Thomas
Cevin Thomas

Reputation: 397

React, writing handleInputChange function more elegant

I have this code that happens quite often in React (handling input from user).

const handleEndDateChange = ( e ) => {
        if ( e.target.name === "month-selected" ) {
            setChosenEndDate( {
                ...chosenEndDate,
                month: e.target.value
            } );
        }
        if ( e.target.name === "year-selected" ) {
            setChosenEndDate( {
                ...chosenEndDate,
                year: e.target.value
            } );
        }
        if ( e.target.name === "day-selected" ) {
            setChosenEndDate( {
                ...chosenEndDate,
                day: e.target.value
            } );
        }
    };

My question is: Is there a nicer and more elegant way to write this? Not only for React, but in general, when it comes to functions that handles these types of actions.

I have recently read "Clean Code" book, and in it it mentions 2 things I keep thinking about.

1: You can tell someone is junior by the overuse of IF statements. 2: A function should only do ONE thing (This function as i showed you, is doing 2 things. First, determining the input, and then setting state).

Upvotes: 1

Views: 310

Answers (4)

Max
Max

Reputation: 4739

The 3 things this function does at once (but shouldn't do) are:

  1. extracting name from event
  2. mapping name to period (year, month, day)
  3. setting it

Reasons for functions to be doing one thing is mainly them being reusable and easy to argue about. In your case, for example, you can't reuse it to set chosen end date programmatically, because there is no event generated. If anything goes wrong, you can't tell whether the problem is in the mapping of names, or assigning values, or reading names from event targets. If you change your event target's name you will have to go to this function and change them here as well. And better hope you don't forget to do it

Consider this example instead

const handleEndDateChange = (period, value) => {
  setChosenEndDate({
    ...chosenEndDate,
    [period]: value
  })
}

In this case it does only one thing: writing value for respective period. Logic of assigning periods to onChange events belongs to your components themselves, e.g.

// somewhere in render()
<MonthItem onChange={e => handleEndDateChange('month', e.target.value)} />
<YearItem onChange={e => handleEndDateChange('year', e.target.value)} />
<DayItem onChange={e => handleEndDateChange('day', e.target.value)} />

In this case onChange prop for your items knows which period it's responsible for, and it is responsible for mapping it's name (e.g. "month-selected", if you still need it) to key in your end date ("month")

Upvotes: 1

SuleymanSah
SuleymanSah

Reputation: 17858

The most elegant way to do is like this:

const handleEndDateChange = e => {
  setChosenEndDate({
    ...chosenEndDate,
    [e.target.name]: e.target.value
  });
};

Your input fields must have name attribute.

Another alternative with destructuring:

const handleEndDateChange = e => {
  const {name, value} = e.target;
  setChosenEndDate({
    ...chosenEndDate,
    [name]: value
  });
};

Upvotes: 6

Piotr Żak
Piotr Żak

Reputation: 2132

const handleEndDateChange = ( e ) => {
       setChosenEndDate( {
              ...chosenEndDate,
              [e.target.name]: e.target.value
             });
            }

in this case: "month-selected" - "year-selected" - "day-selected" are [event.target.name]


It works? If not please comment and we together find a solution.

Upvotes: 3

akhtarvahid
akhtarvahid

Reputation: 9769

Your input field must have name attribute to identify uniquely.

const handleEndDateChange =(e)=> {
  const {name, value} = e.target;
  setChosenEndDate({
    ...chosenEndDate,
    [name]: value
  });
};

Upvotes: 1

Related Questions