6
\$\begingroup\$

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?

\$\endgroup\$
  • \$\begingroup\$ Just to be absolutely sure, are you saying that _hasDataTypeA can throw an exception, or did you mean that it can return false? \$\endgroup\$ – konijn Oct 30 '14 at 14:41
  • \$\begingroup\$ @konijn Yes, I'm saying _hasDataTypeA could, hypothetically, throw an exception. \$\endgroup\$ – aebabis Oct 30 '14 at 15:53
  • \$\begingroup\$ So what should happen in that case ? \$\endgroup\$ – konijn Oct 30 '14 at 16:11
  • \$\begingroup\$ @konjin (I think) I would like a Promise to catch the error so that the caller of _getData can see it by calling .catch, but I'm not entirely confident in this premise either. \$\endgroup\$ – aebabis Oct 30 '14 at 16:16
  • \$\begingroup\$ @konijn Did you want to try to put an answer? You can get some bounty even if the answer isn't accepted. \$\endgroup\$ – aebabis Nov 1 '14 at 18:02
2
\$\begingroup\$

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]();
\$\endgroup\$
  • \$\begingroup\$ getTypeAData and getTypeBData return promises, so you don't need to wrap them at the end. \$\endgroup\$ – aebabis Nov 5 '14 at 16:06
  • \$\begingroup\$ I call then on the return value of SomeBackendAccessObject.getTypeAData(). \$\endgroup\$ – aebabis Nov 5 '14 at 21:46

Your Answer

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

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