current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite

I wrote connection logic for a wrapper around RethinkDB in node.js. While it works, I know it can be cleaned up, and done in a better manner. Essentially the logic is start at the first host in config.hosts, if _connect() fails, then try the next host in the array. This continues for three times, or until config.hosts is empty. If __connect() is successful just set rethinkdbConnection to the returned connection object.

var config.hosts = ['10.0.0.1', '10.0.0.2', '10.0.0.3'];
var rethinkdbConnection;

exports.connect = function(config, callback) {
    var rethinkdbConfig = clone(config);
    delete rethinkdbConfig.hosts;

    function _connect(callback) {
        rethinkdbConfig.host = config.hosts.shift();

        r.connect(rethinkdbConfig, function(error, connection) {
            if(error) {
                return callback(error);
            }

            return callback(null, connection);
        });
    }

    _connect(function(error, connection) {
        if(error) {
            if(!config.hosts.length) {
                return callback(error);
            }

            _connect(function(error, connection) {
                if(error) {
                    if(!config.hosts.length) {
                        return callback(error);
                    }

                    _connect(function(error, connection) {
                        if(error) {
                            return callback(error);
                        }
                    });
                }
            });
        } else {
            rethinkdbConnection = connection;
            return callback();
        }
    });
};

What is a better way to write this?

Also it would be great instead of arbitrarily trying _connet() 3 times, to try as many times as the length of config.hosts instead.

share|improve this question

1 Answer 1

up vote 1 down vote accepted

You can use recursive promises. If promises don't exist in your platform, you could use a third party library or a polyfill to do the same thing.

function tryConnecting(config, remainingHosts){

    // Make a copy of hosts that we can shift off stuff. Don't modify config.
    remainingHosts = config.hosts.slice(0);

    // Promises resolve or reject only when we call resolve and recject.
    // When we see that we can still retry, we don't call either.
    return new Promise(function(resolve, reject)){

        // Update host
        config.host = remainingHosts.shift();

        // Try connecting
        r.connect(config, function(error, connection) {

            // If it fails and we still have hosts remaining, call the function
            // again. This promise's resolve and reject will be called when that
            // call's promise resolves or rejects
            if(error && remainingHosts.length) tryConnecting(config, remainingHosts).then(resolve, reject);

            // If it's an error, and we have no more remaining hosts, we reject
            // the promise. If this call was not the first (ie. previous call
            // failed with remaining hosts), then that promise will be rejected
            // with the same values.
            else if(error && !remainingHosts.length) reject(error, null);

            // If no error, then we resolve with the connection. If this was
            // not the first call (ie. previous call failed with remaining
            // hosts) then, that promise will resolve with the same values.
            else resolve(null, connection);
        });
    });
}

exports.connect = function(config, callback) {
    tryConnecting(config, config.hosts).then(callback, callback);
};
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.