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
    Commented Jul 26, 2016 at 10:15
  • \$\begingroup\$ @Sergio Angular's Promise docs.angularjs.org/api/ng/service/$q \$\endgroup\$ Commented Jul 26, 2016 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
    Commented Jul 26, 2016 at 10:34
  • \$\begingroup\$ @Sergio you are right, will be better to place additional conditional for that case. \$\endgroup\$ Commented Jul 27, 2016 at 2:01

1 Answer 1

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\$ Commented Jul 27, 2016 at 1:58

Your Answer

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

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