The prevailing wisdom says that you should keep your functions small and concise, refactoring and extracting functions as necessary. But this hurts the locality of expectations that I have been thinking about. Consider:
function updateUserStatus(user) {
if (user.status == "active") {
$("").appendTo($("#userlist")
.text(user.name)
.attr("id", "user-" + user.id);
} else {
$("#user-" + user.id).remove();
}
}Code like this is generally considered to be terrible – there’s logic for users and their status, mixed in with a bunch of very specific UI-related code. (Which is all tied to a DOM state that is defined somewhere else entirely — but I digress.) So a typical refactoring would be:
function updateUserStatus() {
if (user.status == "active") {
displayUserInList(user);
} else {
removeUserFromList(user);
}
}With the obvious definition of displayUserInList() and removeUserFromList(). But the first approach had certain invariants that the second does not. Assuming you don’t mess with the UI/DOM directly, and assuming that updateUserStatus() is called when it needs to be called, the user will be in the list or not based strictly on the value of user.status. After refactoring there are functions that could be called in other contexts (e.g., displayUserInList()). You can look at the code and see that particular things happen when updateUserStatus() is called, but it’s not as easy to determine what is going to happen when inspect the code from the bottom up. For instance, you want to understand why things end up in
— you search for #userlist but you now get two functions instead of one, and to understand the logic you have to trace that back to the calling function, and you have to wonder if now or in the future anyone else will call those functions. The advantage of the first function is that blocks of code are strict. You execute from the top to the bottom, with clear control structures. When GOTO existed you couldn’t reason so well, but we’ve gotten rid of that! (Of course there are still other exceptions.) It’s not entirely clear what intention drives the refactoring (besides adherence to conventional standards of code beauty), but it’s probably more about code organization than about making the control flow more flexible. Extracting those functions means that you now have the power to make the UI inconsistent with the model, and that hardly seems like a feature. And I have to wonder: are some of these basic patterns of “good” code there because we have poor tools for code organization? We express too many things with functions and methods and classes (and perhaps modules) because that’s all we have. But those are full of unintended semantic meaning. Anyone have examples of languages that have found novel ways of keeping code organized?
So, it’s a great question on modularity where we tend not to have much explicit thinking : down at the smaller granularity (compared to all the patterns for classes etc.) My immediate comment is that if Ian refactored his code like this :
function updateUserStatus() {
var id = "user-"+user.id;
if (user.status == "active") {
addToList("#userlist",user.name,id);
} else {
removeFromList("#userlist",id);
}
}
it would solve most of the problems. In this version we aren’t fussily creating extra functions for tiny fragments of functionality which are only relevant to narrow situations (ie. users, userlists). Now the new functions are more generic and widely applicable. They’re doing enough that it’s worth the overhead of creating them. They’re still usefully hiding the bit of complexity we DON’T want to think about here – the actual jQuery / HTML details of how lists are constructed – but they’re leaving the important details – WHICH list we’re updating and what parts of a user we show – in this locality rather than allowing it to become diffuse across the program. Of course, we can’t prevent another bit of code updating the list itself somewhere. (That’s more a quirk of the HTML environment where the DOM is global. In many analogous cases we could prevent most of the code having unauthorized access to a list simply by making it private within a class.)
Leave a Reply
You must be logged in to post a comment.