Naming Things

Tuesday 2 February 2016 at 08:00 GMT

A couple of weeks ago, I promised to talk about naming.

If you can name a function really well, it probably does one thing and one thing only. This means you've figured out a decent way to separate your concerns, which means that often, the name is really all you need to know. (Expect more on naming in a future post.)

I just opened up Stack Overflow and clicked on the latest question1 to find this code:

function createList() {
    $.ajax({
        type: "POST",
        url: "fetch_registered_list.php?event_id=1",
        contentType: "application/json; charset=utf-8",
        dataType: "json",
        success: function(data) {
            $('#table_data tr').not(':first').remove();
            if (data != '' || data != undefined || data != null) {
                var html = '';
                $.each(data, function(i, item) {
                    html += "<tr><td>" + data[i].sno + "</td>" + "<td>" + data[i].name + "</td>" + "<td>" + data[i].email + "</td>" + "<td>" + data[i].phone + "</td>" + "<td><input class='check' name='" + i +
                        "' type='checkbox'/></td>" + "<td><input class='score' name='" + data[i].email + "' type='number'/></td></tr>"
                })
                $('#table_data tr').first().after(html);
            }

        }
    });
}

Look at the contents, and then look at the name of the function: createList. Does that resonate with you?

Let me enumerate the things I think this function's doing. Of course, I could be wrong. One of the issues with legacy code is that we often are.

Right. Let's think of a better name for this.

Go on. Take your time. There's no rush.

When you're ready, scroll down and I'll give you mine.

...

Ready?

Really?

You sure? No peeking.

Alright, here we go.

Mine is fetchRegisteredUsersAndRenderToTableAfterClearingOldData.

Mouthful, isn't it? You may have had something similar. I think this is a good name, because it explains what the function does. While retrieveRegisteredUsers might be a more pleasant name, I don't like it, because that kind of information hiding doesn't make my code cleaner, it just makes it more obtuse. With a name like retrieveRegisteredUsers, I might not realise that it's asynchronous behaviour, and so I might invoke it and then something that depends on the result immediately afterwards. I definitely won't realise that it's updating a table.

"Updating a table". I like that. It's more succinct. OK, how about fetchRegisteredUsersAndUpdateTable? That's a bit better. It captures that the code does two things, which is one thing too many. That's good. Now we have a goal. Figure out how to split this so that the two things are done separately. I think the easiest way would be to pull the success function out:

function fetchRegisteredUsersAndUpdateTable() {
    $.ajax({
        type: "POST",
        url: "fetch_registered_list.php?event_id=1",
        contentType: "application/json; charset=utf-8",
        dataType: "json",
        success: updateRegisteredUsersTable
    });
}

function updateRegisteredUsersTable(data) {
    $('#table_data tr').not(':first').remove();
    if (data != '' || data != undefined || data != null) {
        var html = '';
        $.each(data, function(i, item) {
            html += "<tr><td>" + data[i].sno + "</td>" + "<td>" + data[i].name + "</td>" + "<td>" + data[i].email + "</td>" + "<td>" + data[i].phone + "</td>" + "<td><input class='check' name='" + i +
                "' type='checkbox'/></td>" + "<td><input class='score' name='" + data[i].email + "' type='number'/></td></tr>"
        })
        $('#table_data tr').first().after(html);
    }
}

Disregarding the innards of each function for now, that's a little better. Now, what if we inject that callback?

function fetchRegisteredUsers(onSuccess) {
    $.ajax({
        type: "POST",
        url: "fetch_registered_list.php?event_id=1",
        contentType: "application/json; charset=utf-8",
        dataType: "json",
        success: onSuccess
    });
}

We're able to invoke it like this:

fetchRegisteredUsers(updateRegisteredUsersTable);

Great. Two functions, one purpose each, totally decoupled. I'm much happier. And with a little syntactic sugar in the form of JavaScript promises, we can make it read really nicely.

function fetchRegisteredUsers() {
    return Promise.resolve($.ajax({
        type: "POST",
        url: "fetch_registered_list.php?event_id=1",
        contentType: "application/json; charset=utf-8",
        dataType: "json"
    }));
}

...

fetchRegisteredUsers().then(updateRegisteredUsersTable);

Marvellous.

Footnotes

  1. I'm really sorry to the author of this code. I don't mean to pick on them—it really was at random, and I honestly wouldn't have an issue with this code as-is most of the time. Code on Stack Overflow is licenced under Creative Commons Attribution-ShareAlike 3.0 Unported.


If you enjoyed this post, you can subscribe to this blog using Atom.

Maybe you have something to say. You can email me or toot at me. I love feedback. I also love gigantic compliments, so please send those too.

Please feel free to share this on any and all good social networks.

This article is licensed under the Creative Commons Attribution 4.0 International Public License (CC-BY-4.0).