CODEWITHSUNDEEP
javascriptreactjsdesign-patterns
Josh Broadhurst
Josh Broadhurst

Reputation: 439

Simplifying repeated code in React

It there a best practice to simplify something like the following in React?

getInitialState: function () {
  return {
    checkbox1: false,
    checkbox2: false,
    checkbox3: false,
    ...
  };
},

selectCheckbox1: function () {
  this.setState({
    checkbox1: !this.state.checkbox1
  });
},

selectCheckbox2: function () {
  this.setState({
    checkbox2: !this.state.checkbox2
  });
},

selectCheckbox3: function () {
  this.setState({
    checkbox3: !this.state.checkbox3
  });
},

...

render: function () {
  return (
    <div>
      <input type="checkbox" checked={this.state.checkbox1} onChange={this.selectCheckbox1} />
      <input type="checkbox" checked={this.state.checkbox2} onChange={this.selectCheckbox2} />
      <input type="checkbox" checked={this.state.checkbox3} onChange={this.selectCheckbox3} />
      ...
    </div>
  );
}

For example, I can use an array for the state instead of individual fields and create a generic function that takes an index parameter to distinguish which checkbox to update:

const Checkboxes = React.createClass({
  getInitialState: function () {
    return {
      checkboxes: [false, false, false, ...]
    };
  },

  selectCheckbox: function (index) {
    let checkboxes = this.state.checkboxes.slice();
    checkboxes[index] = !checkboxes[index];
    this.setState({
      checkboxes: checkboxes
    });
  },

  render: function () {
    return (
      <div>
        <input type="checkbox" checked={this.state.checkboxes[0]} onChange={() => this.selectCheckbox(0)} />
        <input type="checkbox" checked={this.state.checkboxes[1]} onChange={() => this.selectCheckbox(1)} />
        <input type="checkbox" checked={this.state.checkboxes[2]} onChange={() => this.selectCheckbox(2)} />
        ...
      </div>
    );
  }
});

I am new to React and JavaScript so I don't know exactly the tradeoffs happening behind the scenes here. Is the second an improvement over the first? Which is preferred and why?

Upvotes: 2

Views: 1910

Answers (3)

jonahe
jonahe

Reputation: 5000

I prefer having the checkboxes named (in an object, not an array), like in your first example. But that could vary by use case. Having them unnamed as an array does, as Prakash-sharma points out, provide the benefit of being able to map over them.

This is how I would do it to reduce the duplication of the callback function declarations (without storing the checkbox values in an array):

const Checkboxes = React.createClass({
  getInitialState: function () {
    return {
      checkbox1: false,
      checkbox2: false,
      checkbox3: false
    };
  },

  selectCheckbox: function (checkboxNr) {
    // return a callback with the checkboxNr set
    return () => {
        this.setState({
         [checkboxNr]: !this.state[checkboxNr]
        });
    }
  },

  render: function () {
    const {checkboxes1, checkboxes2, checkboxes3} = this.state;
    return (
      <div>
        <input type="checkbox" checked={checkboxes1} onChange={ this.selectCheckbox("checkbox1") } />
        <input type="checkbox" checked={checkboxes2} onChange={ this.selectCheckbox("checkbox2") } />
        <input type="checkbox" checked={checkboxes3} onChange={ this.selectCheckbox("checkbox3") } />
      </div>
    );
  }
});

Upvotes: 1

thedude
thedude

Reputation: 9814

I would do something like this: 

class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      checkboxes: {
        cheese: false,
        lettuce: false,
        tomatoe: false
      }
    };
  }

  handleChange = e => {
    const checkboxId = e.target.id;
    this.setState({
      checkboxes: {
        ...this.state.checkboxes,
        [checkboxId]: !this.state.checkboxes[checkboxId]
      }
    });
  };

  render() {
    return (
      <div>
        {Object.entries(this.state.checkboxes).map(([key, val]) => {
          return (
            <div key={key}>
              <input
                type="checkbox"
                checked={val}
                id={key}
                onChange={this.handleChange}
              />
              <label>
                {key}
              </label>
            </div>
          );
        })}
      </div>
    );
  }
}

This makes things more explicit and easier to follow and has the added benefit of not creating a new anonymous function each time you render.

Upvotes: 1

Prakash Sharma
Prakash Sharma

Reputation: 16472

Second one is definitely better approach then the first one. You can safely go with that.

Apart from that, you can also render the check box without repeating them each time in render function by using array map.

render: function () {
    return (
      <div>
        {this.state.checkboxes.map((c, i) => {
             return (
              <input key={i} type="checkbox" checked={c} onChange={() => this.selectCheckbox(i)} />
           );
         })}
      </div>
    );
  }

Upvotes: 3

Related Questions