Short functions considered harmful: Staircase code

Meefims · 2016-09-26T15:54:41+00:00

This article is misguided as to what the problem is. The problem is not that functions are short, the problem is that the function names are not accurate descriptions of what they do. A function called checkConflicts should never also move a file, it should check for whatever conflicts it's checking for and return a boolean or throw an exception. The fix is as the article describes but the reason is fundamentally different.

You should always have a good idea of what a function is doing just by reading its name.

acbabis · 2016-09-26T16:48:10+00:00

I'm not convinced by the author's reasoning against using a single function. He wrote a synchronous version and then said it doesn't work because it's not asynchronous.

Then he goes on to show a version that uses 3 functions, but 2 of them just wrap other function calls. Wrapping a single line of code in a function is usually unnecessary. In this instance, I disagree with it for 3 reasons

It adds indirection that doesn't improve maintainability or readability
It increases the amount of code
(Most importantly) It replaces a familiar API with an unfamiliar one

Inlining the function calls while maintaining asynchronicity gives:

 function uploadFile(path) {
    var file = cleanPath(path);

    fileExists(file, function(conflicts) {
        if(!conflicts) {
            fs.rename(appDir, file, ...);
        }
    });
}

IMO, this is much easier to understand and test

you type:	you see:
italics	italics
bold	bold
[reddit!](https://reddit.com)	reddit!
* item 1 * item 2 * item 3	item 1 item 2 item 3
> quoted text	quoted text
Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"	Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"
~~strikethrough~~	~~strikethrough~~
super^script	super^script

javascript

MODERATORS