2
\$\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\$
  • 1
    \$\begingroup\$ Got it! Thanks for the advice - I'm new here but will make sure to adapt \$\endgroup\$ – durandamien1997 Jan 9 at 10:41
2
\$\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\$
  • \$\begingroup\$ Awesome thanks! I didn't know about the bind() method - good to know! \$\endgroup\$ – durandamien1997 Jan 9 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\$ – durandamien1997 Jan 9 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\$ – 409_Conflict Jan 9 at 22:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.