3
\$\begingroup\$

Pretty straight forwards really...I know that it's usually bad practice to repeat your code so I'm wondering how I could join handleAuthorChange() and handleContentChange() into one method. I guess it was hard for me to find a solution because I don't understand how I would know which part of the state I'm updating. I was thinking maybe embed a HTML id and check on the event target if IDs match but that seems pretty inefficient. Thanks in advance!

import React, { Component } from 'react';

class AddComment extends Component {
    state = {
        author: '',
        content: ''
    }

    handleAuthorChange = (e) => {
        this.setState({
            author: e.target.value
        });
    }

    handleContentChange = (e) => {
        this.setState({
            content: e.target.value
        });
    }

    handleSubmit = (e) => {
        e.preventDefault();
        this.props.addComment(this.state);
        this.setState({
            author: '',
            content: ''
        });

    }

    render() {
        return (
            <div className="add-form">
                <form onSubmit={ this.handleSubmit } className="card hoverable p-bottom">
                    <p className="p-top">ADD COMMENT</p>
                    <input onChange={ this.handleContentChange } value={this.state.content} type="text" placeholder="comment" />
                    <input id="name" onChange={ this.handleAuthorChange } value={this.state.author} type="text" placeholder="name" />
                    <input value="submit" type="submit"/>
                </form>
            </div>
        )
    }
}

export default AddComment;
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

I find it fine as it is, but if you really want to keep a single method, you can parametrize it by the name of the key to set:

handleChange = (keyName, e) => { this.setState({ [keyName]: e.target.value }); }

and call it like handleChange('content', event) or handleChange('author', event).

Problem is, now, to call it properly when creating your components. You could write things like:

<input onChange={(e) => this.handleChange('content', e)} value={this.state.content} type="text" placeholder="comment" />

But this syntax creates a new function each time the component is rendered, potentially triggering child re-render as well. React recommends to bind functions in the constructor instead:

import React, { Component } from 'react';

class AddComment extends Component {
    constructor(props) {
        super(props);
        this.state = { author: '', content: '' };
        this.handleAuthorChange = this.handleChange.bind(this, 'author');
        this.handleContentChange = this.handleChange.bind(this, 'content');
        this.handleSubmit = this.handleSubmit.bind(this);
    }

    handleChange(keyName, e) {
        this.setState({ [keyName]: e.target.value });
    }

    handleSubmit(e) {
        e.preventDefault();
        this.props.addComment(this.state);
        this.setState({ author: '', content: '' });
    }

    render() {
        return (
            <div className="add-form">
                <form onSubmit={this.handleSubmit} className="card hoverable p-bottom">
                    <p className="p-top">ADD COMMENT</p>
                    <input onChange={this.handleContentChange} value={this.state.content} type="text" placeholder="comment" />
                    <input id="name" onChange={this.handleAuthorChange} value={this.state.author} type="text" placeholder="name" />
                    <input value="submit" type="submit"/>
                </form>
            </div>
        );
    }
};

export default AddComment;
\$\endgroup\$
3
  • \$\begingroup\$ Awesome thanks! I didn't know about the bind() method - good to know! \$\endgroup\$ Commented Jan 9, 2019 at 22:35
  • \$\begingroup\$ Also, In my example, if I were to keep it how it is...should I be using a constructor function and using 'super(props)'?? \$\endgroup\$ Commented Jan 9, 2019 at 22:51
  • 1
    \$\begingroup\$ @user3158670 since the arrow methods automatically binding this are still considered experimental, I prefer to explicitly bind my methods, thus I need a constructor. But your original code is clear enough without it. \$\endgroup\$ Commented Jan 9, 2019 at 22:57

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.