benlau / quickpromise

Quick Promise - QML Promise Library
Apache License 2.0
179 stars 26 forks source link

Add convenience methods to Q #5

Closed nathanielhourt closed 8 years ago

nathanielhourt commented 8 years ago

Also add test cases verifying that these methods work as expected.

benlau commented 8 years ago

Hi @nathanhourt ,

Thanks for your patch. I could understand how Q.resolved() works (p.s or it should change Q.promise() to accept a parameter? ). But why it need a Q.rejected()?

nathanielhourt commented 8 years ago

In many cases, it's useful to return an already-rejected promise. Take the following code for instance:

function fetchValue() {
    return remoteApi.getValue().then(null, function(reason) {
        console.log("remoteApi.getValue() failed:", reason);
        return Q.rejected(reason);
    };
}

If the error handler simply returned reason then the promise returned by fetchValue would be resolved to reason which is clearly not the correct behavior.

nathanielhourt commented 8 years ago

As to whether they should be parameters to the constructor, I think not. Using the named functions resolved and rejected makes the code more self-documenting. If Javascript had static typing, it would be OK to pass the result/error directly to the constructor, but with dynamic types I suspect it would cause confusion and invite difficult-to-find bugs.

Take the example function above for instance: if the error handler said return Q.promise(reason); the implied behavior would clearly be that it would return a rejected promise with the supplied reason (particularly since this is QML, and C++ programmers expect that kind of behavior from static typing), but the actual behavior would be a resolved promise with an error as its result. This kind of bug can be nearly impossible to find due to a programmer's tendency to read code as he intended it, not as he wrote it. :)

benlau commented 8 years ago

Hi @nathanhourt ,

I see. It make sense. Let's add those API. Thanks!