Refactoring our Ajax Code: Don't Repeat Yourself

Our code works, there's no doubt about it. And it's easy to read, because we've just taken the basic concepts from earlier and this tutorial and duplicated them three times. But crucially it's not easy to understand or maintain.

The reason it's hard to understand is because you need to read through an awful lot of code to see what it's doing. Yes, we might know the code is pretty much exactly duplicated, but others won't, and they'd have to read through it all to see that.

The reason it's hard to maintain is because the code has literally been copied and pasted, with minor adjustments. What if you find a bug later on – will you remember to change it for commits, pulls and forks, or is it possible you'll forget one of them?

We can fix these problems by refactoring our code, which is a fancy name for a process of making code better. The definition of "better" might be faster to run, easier to understand, shorter, more re-usable, or any number of improvements or combinations of improvements, but hopefully the result is better than what you started with.

An easy target in our current code is the componentWillMount() method. It makes three large Ajax calls, each of which vary by just three lines:

• ajax.get('https://api.github.com/repos/facebook/react/commits')
• this.setState({ commits: response.body })
• console.log('There was an error fetching commits from GitHub', error);

More importantly, those three lines all vary only by three words, and the word is the same each time: commits, commits, commits; forks, forks, forks; pulls, pulls, pulls. This is ripe for refactoring: we could create a method that accepts a string as its only parameter, e.g. "commits", then puts it into those three places. We can then call that method three times in componentWillMount().

To make this work we need some new ES6 syntax: string interpolation and computed property names. I'll show you the code first then explain the interesting bits – please add this method to the Detail component:

src/pages/Detail.js

fetchFeed(type) {
    ajax.get(`https://api.github.com/repos/facebook/react/${type}`)
        .end((error, response) => {
            if (!error && response) {
                this.setState({ [type]: response.body });
            } else {
                console.log(`Error fetching ${type}`, error);
            }
        }
    );
}

So, it's a method called fetchFeed() and it takes a single parameter called type. To place that into the ajax.get() URL I've used ES6 string interpolation: the URL is now wrapped in backticks (those funny angled quotes that usually share a key with ~) rather than single quotes, and when you do that you can place variables (including other expressions) right inside the string. When the compiler sees ${type} it substitutes the contents of the type parameter at that point. The same technique is used in the console.log() statement.

The second ES6 feature in there is called computed property names, and you see it in the call to this.setState(). Take a look at this code:

this.setState({ type: response.body });

Is that saying a) "put response.body in my object using the name type", or b) "put response.body in my object using the name commits because that's what the type parameter is set to"?

The answer is a), and there were some ugly hacks you could do to work around that. With ES6 you can now write [type], which is a computed property name, and effectively tells the computer you mean b).

With the new fetchFeed() method in place, we just need to call it three times when the component mounts. Modify your componentWillMount() method to rip out all that Ajax code and replace it with this:

src/pages/Detail.js

componentWillMount() {
    this.fetchFeed('commits');
    this.fetchFeed('forks');
    this.fetchFeed('pulls');
}

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