HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Using a Wrapper Promise

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
wrapperusingpromise

Problem

I wrote some code for an application that uses the following pattern:

function _getData() {
    return new Promise(function(resolve, reject) {
        if(_hasDataTypeA()) {
            SomeBackendAccessObject.getTypeAData().then(resolve, reject);
        } else {
            SomeBackendAccessObject.getTypeBData().then(_convertToTypeA).then(resolve, reject);
        }
    });
}


My rationale for writing the code this way is that if _hasDataTypeA throws an exception, we're still able to return a Promise. This means the caller doesn't have to do a try block and a Promise .catch; but this code feels smelly to me. Is there a better way to write it?

Solution

First thing I noticed is that your fulfilling the wrapper promise by way of other promises inside it. Seems like and extra step that isn't needed. Especially the final .then(resolve, reject) feels like a big anti-pattern especially since it is a repeated pattern. So the rational is to catch the exception. My take on this is to wrap only _hasDataTypeA in it's own promise and then use that result to manage the following promises:

function _hasDataTypeAPromised() {
  try {
    return Promise.resolve(_hasDataTypeA());
  } catch (e) {
    return Promise.reject(e);
  }
}

function _getData() {
  return _hasDataTypeAPromised()
    .then(function(hasDataTypeA) {
      if(hasDataTypeA) {
        return SomeBackendAccessObject.getTypeAData();
      } else {
        return SomeBackendAccessObject.getTypeBData();
      }
    });
}


You could even ternary-ize the method with:

var method = hasDataTypeA ? 'getTypeAData' : 'getTypeBData';
return SomeBackendAccessObject[method]();

Code Snippets

function _hasDataTypeAPromised() {
  try {
    return Promise.resolve(_hasDataTypeA());
  } catch (e) {
    return Promise.reject(e);
  }
}

function _getData() {
  return _hasDataTypeAPromised()
    .then(function(hasDataTypeA) {
      if(hasDataTypeA) {
        return SomeBackendAccessObject.getTypeAData();
      } else {
        return SomeBackendAccessObject.getTypeBData();
      }
    });
}
var method = hasDataTypeA ? 'getTypeAData' : 'getTypeBData';
return SomeBackendAccessObject[method]();

Context

StackExchange Code Review Q#67461, answer score: 2

Revisions (0)

No revisions yet.