kitschysynq/heavytungsten — pull request #2 by thisgeek (2012-06-13 15:39:40+00:00)
Title: Two Refactors

Two major refactors in this request:

1. Functions with a common namespace (e.g. `delete_notes`) were rewritten as "methods" of an object literal called `notes`. (Note: `delete` is a reserved word in JS, so `destroy` took its place.)
2. The "find a single document by id" concern shared by the `destroy` and `retrieve` methods was factored out into a new method called `fetchOne` (for lack of a better idea). `fetchOne` is a kind of middleware function that alters the state of the response object by adding a `doc` attribute, with the expectation that something later in the route function queue may need it.

Take-aways:

Idiomatically, and at minimum, you should group globally available functions with a common namespace with JS object literals. If you ever move common functions into their own module, the refactor will be clearer if you do this.

Restify's architecture relies on the vistor pattern to keep the code DRY. Use middleware functions to consolidate concerns.

I have caused some confusion by adding a `notes` variable in a scope where we already have `Notes`. That's somewhat intentional. I would expect to find a way to only instantiate `Notes` in the course of a request, rather than when the app is instantiated. I didn't think it through, however, so maybe the way you already have it is easier on the resources. If so, consider renaming `notes` to `notesMiddleware` or `notesController`. Ping me if you'd like to discuss.