Node.js 'request' module wrapper with optional params - how to write this more concise?

Question

I'm writing a wrapper around node-request:

var RequestWrapper = function(client, method, type, uuid, body, callback) {

    callback = callback || body || uuid

    // Let's get this looking a little cleaner!
    body = (typeof body === 'object') ? body : uuid
    body = (typeof body === 'object') ? body : undefined

    var uri = urljoin(config.baseUrl, type, (typeof uuid === 'string') ? uuid : "")
    var params = request.initParams(uri, {
        body: body
    }, callback)

    params.method = method

    params.json = true

    request(params, function(error, response) {
        response = new UsergridResponse(response)
        params.callback(error, response)
    })
}

module.exports = RequestWrapper

I'm supporting four RESTful methods, with optional arguments like so:

wrapper(client, 'GET', 'pets', cb) // returns an array of pets
wrapper(client, 'GET', 'pets', '<uuid>', cb) // returns a specific pet
wrapper(client, 'PUT', 'pets', '<uuid>', cb) // updates the specified pet
wrapper(client, 'POST', 'pets', { body: {} }, cb) // creates a new pet
wrapper(client, 'DELETE', 'pets', '<uuid>', cb) // deletes the specified pet

My RequestWrapper works like it is, but I really dislike the way I'm checking for optional body param and optional uuid param. I managed to get callback all pretty looking -- is there a way I can do this for body too?

@Shashank yeah I am, though I'd like to see this in a non-ES6 solution too. — brandonscript, Nov 23 '15 at 20:48
This question belongs on http://codereview.stackexchange.com/ because it is about how to write working code _differently_. — Evan Davis, Nov 23 '15 at 20:51
Off topic: Regarding your question you just deleted, you'd be much better off using something like this to get/test deep properties: http://stackoverflow.com/questions/6491463/accessing-nested-javascript-objects-with-string-key — Evan Davis, Nov 24 '15 at 19:27
@Mathletics yeah - deleted it because I realized I was being a knob. I misunderstood their function name - that one is ONLY for functions. I want to do it for properties, so need to rethink how to ask. Related: https://github.com/letsgetrandy/brototype/issues/38#issuecomment-159378582. Thanks for your insight though ;) — brandonscript, Nov 24 '15 at 19:28

Evan Davis · Accepted Answer · 2015-11-23T20:49:19.577

2

For this many parameters, use an options hash.

var RequestWrapper = function(opts) {
    var uri = urljoin(config.baseUrl, opts.type, (typeof opts.uuid === 'string') ? opts.uuid : "")
    var params = request.initParams(uri, {
      body: opts.body
    }, opts.callback)
/* etc */

in ES6 you can use destructuring to make this even cleaner.

var RequestWrapper = function(opts) {
    let {client, method, type, uuid = '', body, callback} = opts; // default for uuid
/* etc */

edited Nov 23 '15 at 20:49

answered Nov 23 '15 at 20:47

Evan Davis

35,493
6
50
57

This requires me to pass an options object to the method though right? Would prefer not to do that. – brandonscript Nov 23 '15 at 20:48
I don't know why you would want to manage that many ordered params, but ok. – Evan Davis Nov 23 '15 at 20:51
I suppose you're right Also of note though, uuid isn't the problem I'd like to solve, it's body. So I can do the same thing, but I still have to check multiple properties to see if body is the correct one. – brandonscript Nov 23 '15 at 20:52
The other option would be to break up the requests by type, creating `index`, `show`, `edit`, etc functions that take fewer params. It would also look way less goofy when called (IMO). – Evan Davis Nov 23 '15 at 20:55
No, `body` would always be `opts.body`. You pass it like `RequestWrapper({body: {whatever: {}})` so you don't have to inspect it. Or if you do, it's just `if (opts.body)` (objects are truthy so you know a `body` was passed.) – Evan Davis Nov 23 '15 at 20:56
Yeah, which means I'm stuck passing an object. Trying to be as consistent as possible to [tag:node-request] which supports both. I'll have to move the ordered params up a level and do the checking further down the chain. Not a big deal, but definitely cleaner. – brandonscript Nov 23 '15 at 20:59
_Trying to be as consistent as possible to node-request_ I don't understand what you mean by that? – Evan Davis Nov 23 '15 at 21:01
`request` supports ordered params: `request(uri, options, cb)` or an object `request({uri: url, body: body, headers: headers}, cb)`. Trying to build this wrapper with that same structure in mind so that this can be a drop-in replacement. – brandonscript Nov 23 '15 at 21:03
So take a hash and a callback (2 params) so that it matches theirs. – Evan Davis Nov 23 '15 at 21:05
Yeah, that's what I'm doing. – brandonscript Nov 23 '15 at 21:08

Shashank · Answer 2 · 2015-11-24T01:29:05.793

This may not be what you want but you could try using rest parameters in combination with Array.find. Unfortunately rest parameters are an ES6 language feature so you may need to use Babel or the --harmony flag.

var RequestWrapper = function(client, method, type, ...opts) {
  const isType = (t) => (x) => typeof x === t,
        uuid = opts.find(isType('string')) || '',
        body = opts.find(isType('object')),
        callback = opts.find(isType('function'))
  // . . .
}

Working with it as an array makes it slightly cleaner and rest parameters are nice syntax sugar for automatically converting tailing arguments to arrays. In ES5, you could slice the arguments from index 3 and work with it that way.

To get even fancier with function mapping and fail-soft destructuring:

const firstOptOfType = (t) => opts.find((opt) => typeof opt === t),
      [uuid = '', body, callback] = ['string', 'object', 'function']
                                      .map(firstOptOfType)

Node.js 'request' module wrapper with optional params - how to write this more concise?

2 Answers2