7
\$\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\$
5
  • \$\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, 2014 at 14:41
  • \$\begingroup\$ @konijn Yes, I'm saying _hasDataTypeA could, hypothetically, throw an exception. \$\endgroup\$
    – aebabis
    Oct 30, 2014 at 15:53
  • \$\begingroup\$ So what should happen in that case ? \$\endgroup\$
    – konijn
    Oct 30, 2014 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, 2014 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, 2014 at 18:02

1 Answer 1

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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