1
\$\begingroup\$

This JavaScript code is being used as part of a reporting tool that makes http(s) GET requests to an API, bundles responses and uploads them as files to an AWS S3 bucket. It's a script that runs periodically to save changes in our API over time. This particular function getResponse (name pending) returns a promise, that makes the actual requests with either node's http or https library, depending on the passed in url string.

const DEBUG = process.env.DEBUG;

const getResponse = url => {
  return new Promise((resolve, reject) => {
    (() => url.startsWith('https://') ? require('https') : require('http') )().request(url, (resp) => {
      let data = '';
      resp.setEncoding('utf8');
      resp.on('data', chunk => data += chunk);
      resp.on('error', err => { if (DEBUG) console.log('fucked up'); return reject(err); });
      resp.on('end', () => { if (DEBUG) console.log(url + ' resolved'); return resolve(data); });
    }).end();
  });
};

Originally I had both the http and https module declared as constants on top of the file and use the variable names in the ternary operation as listed below:

// top of the file
const http = require('http');
const https = require('https');

// later in the method getResponse...
    const r = url.startsWith('https://') ? https : http;
    r.request(url, (resp) => {
// and so on...

When this script is run, the urls that get passed to the function in question are either all starting with https:// or http://, depending which environment we're hitting. Therefore requiring both http and https for the lifetime of the node script is unnecessary. I'm wondering though if my new variation using an anonymous function and requiring in the ternary operation has any disadvantages.

\$\endgroup\$
  • \$\begingroup\$ Can't see why you need to wrap the ternary in an IIFE. Surely brackets would suffice? \$\endgroup\$ – Roamer-1888 Nov 12 '16 at 7:39
  • \$\begingroup\$ Also, if you are 100% confident that all urls will have the same protocol as the first, then you can cache require('https') or require('http') in an outer var .... var protocol = null;, then (protocol || (protocol = ... ? ... : ...)).request(...).end();. \$\endgroup\$ – Roamer-1888 Nov 12 '16 at 8:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.