0

I have combined some typical (anti-)pattern I have come along, looking at javascript using the Q library:

How much of this can be simplified? (or is pointless) Especially:

  • A: What is the value in deferred.resolve(... over returning a value and thus resolving right away? Only the fact, that code that further code get's executed, ie. doSomeMore() ?
  • if so, for A2 this is pointless in this code?
  • B: What is the value in deferred.reject over throw throw new Error('')
  • C: The try-catch also irritates me. Only to ensure doSomeMore() ?

    function foo(u) {

    var deferred = Q.defer();
    
    bar(u).then( function (v) {
    
        if (v==42) {
            deferred.resolve( Q(null) );    // A
        } else {
            try {
                var controller = new Controller();
                deferred.resolve( controller ); // A2
            } 
            catch (error) {
                deferred.reject(error);     // B,C
            }
    
            doSomeMore();
        }
    
        return deferred.promise;
    });
    

    }

4
  • I'm confused as to what this code is supposed to do. Also, this code will not even parse; there's a missing end parenthesis for the then(. May 19, 2016 at 14:25
  • 1
    Are you saying that you have combined the antipatterns on purpose?
    – Bergi
    May 19, 2016 at 14:31
  • @Bergi: I have found them in existing code and damped down to focus on the relevant parts of issues I see...
    – Frank N
    May 19, 2016 at 15:27
  • @MikeMcCaughan corrected the ). It's not supposed to do much, beyond fulfill (sucessfully resolve) to a Controller or Null under some circumstances, to rejection under others. — I encounter such thing things (in less abstract form of course) in production code, thus seeking a better understanding for (anti)patterns and practices...
    – Frank N
    May 19, 2016 at 15:33

2 Answers 2

1

Yes, the purpose of this code seems to be ensuring that doSomeMore() is executed regardless what happened when the Controller was constructed. You could get that more easily though:

function foo(u) {
    return bar(u).then(function (v) {
        if (v == 42) {
            return null;
        } else {
            try {
                return new Controller();
            } finally {
                doSomeMore();
            }
        }
    });
}
3
  • @jfriend00: Yes, finally does always execute, that's its entire point :-)
    – Bergi
    May 19, 2016 at 15:13
  • return null is just as good as deferred.resolve( Q(null) );? (That's what I was thinking, too)
    – Frank N
    May 19, 2016 at 15:34
  • @FrankN: Yes, pretty much (assuming that deferred doesn't matter)
    – Bergi
    May 19, 2016 at 15:35
0

Something like this maybe?

return bar(u).then(function (v) {      
  if (v==42) 
    return null;  
  return new Controller();  
}).finally(function() { doSomeMore(); });
1
  • As @Bergi points out this code misses the doSomeMore() only executing when v != 42. May 19, 2016 at 14:41

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.