Refactoring our State Code: Passing Parameters in onClick

The next thing that should jump out at you for refactoring ought to be the three methods showCommits(), showForks() and showPulls() – they all just change the state to one of three strings, so this should be fairly easy to refactor.

Well, yes: it is easy to refactor. But it also gives me a chance to show you two different ways of sending data to methods. Right now all you've used is either onClick={this.someMethod} or onClick={this.someMethod.bind(this)} – no interesting parameters have been passed.

We want to send along either 'commits', 'forks' or pulls' depending on which button was clicked, which is easy enough. Update your render() code to this:

src/pages/Detail.js

return (<div>
    <button onClick={this.selectMode.bind(this, 'commits')}>Show Commits</button>
    <button onClick={this.selectMode.bind(this, 'forks')}>Show Forks</button>
    <button onClick={this.selectMode.bind(this, 'pulls')}>Show Pulls</button>
    {content}
</div>);

That makes all three buttons call the same method, so it's now just a matter of writing the selectMode() method so that it accepts a parameter and uses it to set the mode state:

src/pages/Detail.js

selectMode(mode) {
    this.setState({ mode });
}

Note: you don't need to use the new ES6 computed property name syntax here, because you've always been able to use variables as values. In fact, because the key and value are the same, we can just write mode.

With selectMode() in place, you can go ahead and delete showCommits(), showForks(), and showPulls().

That code works, and it works well. But we could rewrite it slightly differently, and I'm going to show it to you because it's the kind of thing you'll find in real code, not because I'm saying I favor one approach over the other. There are two ways of doing it, and two camps of people who each are convinced their way is the One True Way, but again I suggest you try to be pragmatic.

IMPORTANT WARNING: I am going to show you how this looks just so you're aware of it. You should keep using your existing code rather than switch to this alternative.

The other way we could write these onClick is by storing data in the buttons that describe what they should do. The selectMode() method can then read that data and act appropriately. To take this approach, we would need to modify the render() method to this:

return (<div>
    <button onClick={this.selectMode.bind(this)} data-mode="commits">
        Show Commits
    </button>

    <button onClick={this.selectMode.bind(this)} data-mode="forks">
        Show Forks
    </button>

    <button onClick={this.selectMode.bind(this)} data-mode="pulls">
        Show Pulls
    </button>

    {content}
</div>);

(Note: I split the button elements onto multiple lines to make them easier to read; you can write them on one line if you prefer.)

As you can see, that no longer passes a parameter string to the selectMode() method. Instead, the strings are stored inside data-mode parameters. To make selectMode() work with this relies on a JavaScript implementation detail: all event handlers are automatically passed an event object describing what happened. We haven't been using this so it's been silently ignored. But in code that uses this data-mode attribute approach we would – here's how the selectMode() method would need to look:

selectMode(event) {
    this.setState({ mode: event.currentTarget.dataset.mode });
}

As you can see, to read the data-mode attribute of whichever button was clicked, we just read the dataset.mode property of the event's currentTarget – that will automatically be the clicked button.

There are good reasons to use both of these ways of calling methods. Explicitly passing parameters makes your code a bit easier to read because you can see exactly what is being sent and what is being received, but having methods pull data from the event can be helpful to reduce code duplication. Again, be pragmatic!

REMINDER OF IMPORTANT WARNING: Code much later in this book relies on you passing a string parameter to selectMode() rather than using the data-mode attribute approach.

That's enough refactoring for now. If it were my own code, I'd probably try to harmonize the various rendering methods a little, but it's not something we can do here because you've probably chosen different JSON fields to me. Still, consider it an exercise: can you get down from four rendering methods to three, two or even one? You might need to clean the JSON before you use it for your component's state.

Copyright ©2016 Paul Hudson. Follow me: @twostraws.