Nice, I like. Thanks for taking the time to explain your changes. Perhaps toss a summary of this into the style guide wiki article for JS shizzle?
On 5/4/12 9:39 AM, "Patrick Mueller" <[email protected]> wrote: >As part of CB-634 [1], I've done some refactoring of the source of the >commons/util.js module. The refactoring is based on some patterns I've >picked up both perusing other people's node modules and writing my own. > Totally subjective, so ... please let's discuss if you disagree. > >The utils module follows what I will call the "module as a bag of >properties" pattern. It's a module that exports an object which has a >bunch of properties in it - where the property values are typically >functions. > >eg, you use the new format() function from utils in your own module, like >so ... > > var utils = require("{whatever}/util") > ... > > var string = utils.format("%s: %o", name, object) > >So, what's changed from the original utils source? Note that I have not >changed how the module works/appears externally, just the source code that >defines the module. > > new: http://bit.ly/Jj6DUh > old: http://bit.ly/IAutNv > >- UUIDcreatePart() function moved to the bottom. This function is a >module-local function, it's only used directly by other functions in the >module. It's implementation detail. It shouldn't be at the top of the >module, it should be at the bottom, so the primary API of the module can >be >at the top. > >- the old module used the "_self object literal" pattern (see further >points below) > >- using "_self" it makes it sound more OO than it should be. The "module >as a bag of properties" pattern really boils down to exposing the contents >of a module as a LIBRARY of functions behind a namespace (utils), instead >of an OBJECT. Libraries are great ways to expose function, when it fits. > Objects tend to be a rather horrible way to expose function, unless you >have to. Libraries > Objects. This is just a mind-set thing. > >- instead of "_self", I've seen many modules use a local variable which >matches how users typically use the module. ie, the variable should be >named "util" instead of "_self" > >- instead of creating a new object containing your exported >properties/functions, just use the object in the "exports" variable that >the runtime has already given you. If you create a new object, as the old >code did, then you need to also remember to do a "module.exports = >myNewExportObject" somewhere. If you reuse the existing "exports" object, >you don't need to. In general, you should avoid using the "module.exports >= ..." trick/hack, unless you really need to (technically: causes issues >in >recursive require() situations). > >- to both reuse the existing exports object AND be able to name it the >name >of your module, use the following assignment at the top of your module: > > var <myModuleName> = exports > >- now you can define all the things you're exporting, like this: > > <myModuleName>.foo = function(bar){} > >- bonus, since we're not creating a new object, we don't need to do the >horrifying "define your properties which include functions inside an >object >literal" thing. That anti-pattern means nesting your junk another level >in, dealing with another outer layer of {}, and the worse part, having to >remember to put a comma BETWEEN all your property/value pairs BUT NOT ADD >ONE AT THE END OMG!!! > >- I also added a comment between each function - both exposed functions >and >local functions. Exposed functions should have structured comments, local >functions should be self-descriptive code but still have some kind of >comment separator to aid in readability. > >Whatcha think? > >[1] CB-634 - need an sprintf implementation in utils > https://issues.apache.org/jira/browse/CB-634] > >-- >Patrick Mueller >http://muellerware.org
