Reducing code complexity for UsergridClient constructor

Question

I'm initializing a class in JavaScript with a bunch of complex property checks. Codacy is showing that this is bad design and recommends reducing cyclomatic complexity.

I'm at a loss for how to improve/streamline this, though.

var ok = require('objectkit')

var UsergridClient = function(opts) {
    var self = this

    // required properties
    if (arguments.length === 2) {
        self.orgId = arguments[0]
        self.appId = arguments[1]
    } else {
        self.orgId = ok(opts).getIfExists('orgId') || ok(config).getIfExists('usergrid.orgId')
        self.appId = ok(opts).getIfExists('appId') || ok(config).getIfExists('usergrid.appId')
    }

    // optional initializer properites
    self.baseUrl = ok(opts).getIfExists('baseUrl') || ok(config).getIfExists('usergrid.baseUrl') || 'https://api.usergrid.com' // default: https://api.usergrid.com
    self.clientId = ok(opts).getIfExists('clientId') || ok(config).getIfExists('usergrid.clientId') // default: undefined
    self.clientSecret = ok(opts).getIfExists('clientSecret') || ok(config).getIfExists('usergrid.clientSecret') // default: undefined
    self.tokenTtl = ok(opts).getIfExists('tokenTtl') || ok(config).getIfExists('usergrid.tokenTtl') || 3600 // time in seconds, default:  3600
    self.authFallback = ok(opts).getIfExists('authFallback') || ok(config).getIfExists('usergrid.authFallback') || 'none' // ('none'|'app') default: 'none'

    // optional post-init properties
    self.paginationPreloadPages = 0 // number of pages to preload
    self.paginationCacheTimeout = 300 * 1000 // default: 300 seconds
    self.paginationCursors = [] // array of pagination cursors

    if (self.orgId !== undefined && self.appId !== undefined) {
        return self
    } else {
        throw new Error('"orgId" and "appId" parameters are required when instantiating UsergridClient')
    }
}

Answer 1

The variable self here is redundant.

The parameter name "opts" is significantly less commonly used than "options".

It is not clear immediately what can be supplied in opts. Inferring how the function can be called from the logic of the code is error-prone.

The code implies that the various optional properties might be in one of two locations, or missing. I would ensure that callers position these values in one location for the call, making testing simpler.

The objectkit library is so verbose as to obfuscate the code quite effectively.

Simply ensure the property exists and negate the need for the getIfExists at all.

Setting default values where none is supplied can be performed easily with Object.assign polyfills and similar libraries.

The following appear to always be the same, if so, do they need to be own-properties?

self.paginationPreloadPages = 0 // number of pages to preload
self.paginationCacheTimeout = 300 * 1000 // default: 300 seconds

The following check should be at the start of the function to short-circuit all the work:

if (self.orgId !== undefined && self.appId !== undefined) {

none and app are magic strings that should be pulled into a single enumeration object.

return self is redundant if the intention is to use this as a constructor function.

So applying these changes:

var AuthFallback = {
    APP: 'APP',
    NONE: 'NONE',
};

var defaultOptions = {
    orgId: null,
    appId: null,
    baseUrl: 'https://api.usergrid.com',
    clientId: null,
    clientSecret: null,
    tokenTtl: 3600,
    authFallback: AuthFallback.NONE,
};

function UsergridClient(options) {
    if (!options || !(options.orgId && options.appId)) {
        throw new Error('options.orgId and options.appId required.');
    }

    Object.assign(this, defaultOptions, options); // or polyfill
}

UsergridClient.prototype.paginationPreload = 0; // pages.
UsergridClient.prototype.paginationCacheLifetime = 300 * 1000; // ms.
UsergridClient.prototype.paginationCursors = []; // Not sure about this one...

Answer 2

With help from Ben, I ended up with this, using underscore.js's defaults (which supports ES5):

var AuthFallback = {
    APP: 'APP',
    NONE: 'NONE',
}

var defaultOptions = {
    baseUrl: 'https://api.usergrid.com',
    authFallback: AuthFallback.NONE,
    paginationPreloadPages: 0, // number of pages to preload
    paginationCacheTimeout: 300 * 1000, // default: 300 seconds
    paginationCursors: [] // array of pagination cursors
}

var UsergridClient = function(options) {
    var self = this

    if (arguments.length === 2) {
        self.orgId = arguments[0]
        self.appId = arguments[1]
    } 

    _.defaults(self, options, ok(config).getIfExists('usergrid'), defaultOptions)

    if (!self.orgId || !self.appId) {
        throw new Error('"orgId" and "appId" parameters are required when instantiating UsergridClient')
    }
}

Much cleaner!

---

(old)

Turns out Codacy definitely didn't like the nested || statements. Adding in a helper function to handle the properties solved the problem:

var UsergridClient = function(opts) {
    var self = this

    // required properties
    if (arguments.length === 2) {
        self.orgId = arguments[0]
        self.appId = arguments[1]
    } else {
        self.orgId = getProperty(opts, 'orgId')
        self.appId = getProperty(opts, 'appId')
    }

    // optional initializer properites
    self.baseUrl = getProperty(opts, 'baseUrl', 'https://api.usergrid.com') // default: https://api.usergrid.com
    self.clientId = getProperty(opts, 'clientId')
    self.clientSecret = getProperty(opts, 'clientSecret')
    self.tokenTtl = getProperty(opts, 'tokenTtl') // token ttl in seconds, server default: 3600
    self.authFallback = getProperty(opts, 'authFallback', 'none') // ('none'|'app') default: 'none'

    // optional post-init properties
    self.paginationPreloadPages = 0 // number of pages to preload
    self.paginationCacheTimeout = 300 * 1000 // default: 300 seconds
    self.paginationCursors = [] // array of pagination cursors

    if (self.orgId !== undefined || self.appId !== undefined) {
        return self
    } else {
        throw new Error('"orgId" and "appId" parameters are required when instantiating UsergridClient')
    }
}

function getProperty(opts, name, defaultValue) {
    return ok(opts).getIfExists(name) || ok(config).getIfExists(util.format('usergrid.%s', name)) || defaultValue
}