1
\$\begingroup\$

Sometimes I need to repeat an execution of rejected promises several times, for example, to fetch some data over internet. There is a wrapper, which accepts Promise and tryCount:

function tryPromise(f, tryCount) {
  return f().then(null, function(v) {
    tryCount--;
    if (tryCount > 0) {
      return $q.reject(v);
    }
    return tryPromise(f, tryCount);
  });
}

Is there some hidden troubles?

\$\endgroup\$
4
  • \$\begingroup\$ What is $q in return $q.reject(v);? \$\endgroup\$
    – Sergio
    Jul 26 '16 at 10:15
  • \$\begingroup\$ @Sergio Angular's Promise docs.angularjs.org/api/ng/service/$q \$\endgroup\$ Jul 26 '16 at 10:25
  • 2
    \$\begingroup\$ if tryCount is <= 0 that code will enter a loop and kill your browser if there is no waiting moment in f.. What numbers to you give tryCount when you invoque that function? will it ever be <=0? \$\endgroup\$
    – Sergio
    Jul 26 '16 at 10:34
  • \$\begingroup\$ @Sergio you are right, will be better to place additional conditional for that case. \$\endgroup\$ Jul 27 '16 at 2:01
3
\$\begingroup\$

Well, first of all you aren't trying a promise. What you're really doing is trying out a function that returns a promise. So the name is already misleading. Try something better.

Next, f and v doesn't really tell me anything. Only when I read through the code did I realize f was the function to try out and v is supposed to be a value. Additionally, even if you name v a value, it's still not correct. Most reject handlers often pass an error object.

Code that requires one to actually read to understand is a bad practice. In this case it's negligible due to the size of the function. But if you're in larger codebases, it's a nightmare to maintain. Name your functions and variables meaningfully.

Lastly, your function doesn't allow for function arguments. It would be nice to at least accept argument 3 onwards as the arguments, or accept an array that would be the arguments.

My take on it would be:

function tryPromiseFunction(functionToTry, retries, ...args){
  return functionToTry(...args).then(null, error => {
    return retries > 0 ? tryPromiseFunction(functionToTry, retries - 1, ...args)
                       : Promise.reject(error);
  });
}
\$\endgroup\$
1
  • \$\begingroup\$ Looks good, many thanks. Not sure, should we compare retries with > 1instead > 0 ? \$\endgroup\$ Jul 27 '16 at 1:58

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.