Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should when.js Promises implement valueOf() and/or toString()? #21

Closed
briancavalier opened this issue Feb 16, 2012 · 9 comments
Closed
Assignees
Milestone

Comments

@briancavalier
Copy link
Member

Implementing valueOf() could be interesting in that it would allow the valueOf a resolved promise to return the resolution value, which would allow some operators, like ==, +, etc. to work on resolved promises. For example, doing a comparison directly on a promise if(resolvedPromise == 1) {...}.

The potential downside is that people might misuse this, or if they aren't familiar with using promises, could be confused into using it incorrectly. For example setInterval(function() { if(promise == 1) doSomething(promise.valueOf()); }, 100)', rather than when(promise, doSomething);

Not sure yet what the value would be in implementing toString() to return something like "[object Promise]" when unresolved, and String(resolvedValue) when it is resolved. But we should at least think through it.

@gamtiq
Copy link

gamtiq commented Feb 17, 2012

Hello Brian,

I agree valueOf() may lead to confusion, misunderstanding and misuse.
IMHO, instead of or along with valueOf it would be useful to have special API to check resolution status and get result. Something like the following:

  • isResolved(): returns true if promise is already resolved.
  • getResult() or getValue(): returns the resolved value if promise is resolved or undefined if it is not.

I suppose toString() implementation might be useful, at least for debug purposes. But I think for resolved promise it would be better to return another string that includes "class" and "properties", for example: [object Promise(resolved;<result>)]

Denis

@briancavalier
Copy link
Member Author

Hey Denis, thanks for the feedback.

I've know that there are other promise libraries, like Q and jQuery, that provide isResolved() and getResult() type functions. I have the same concerns about those, because I think they are essentially the same as valueOf(). Anything that encourages a developer to poll for promise resolution seems bad to me. For example, you could write the same setInterval nonsense using isResolved() or getResult() as you could with valueOf(). I think when.js should encourage (force?) people use when() and/or .then().

That said, maybe there are valid use cases for isResolved() and getResult(). If you have some in mind, let's discuss them here, since that would definitely help with making a decision!

Yep, I agree that toString() could be useful for debugging, and that might be the compelling reason to implement it. Developers could still misuse toString() to poll for resolution, but I think that's less likely, and it doesn't directly encourage polling, like valueOf(), isResolved(), and getResult() would.

@gamtiq
Copy link

gamtiq commented Feb 17, 2012

Brian,

I agree that polling of promise resolution is bad idea and developers should use when().then() approach with proper callbacks. At the moment, I don't have any use cases for isResolved()+getResult() or valueOf().

@briancavalier
Copy link
Member Author

Cool, thanks for the info, Denis!

@briancavalier
Copy link
Member Author

Ok, I'm taking valueOf() off the table for the reasons noted in the discussion above--basically that it encourages polling instead of using when() or .then(). I think toString() could still be reasonable, but now that when.js promises share a constructor, it's pretty obvious in any reasonable debugger that an object is a promise. So, I don't see this as a high priority for now ... I'll probably let it ride until we find a convincing use case.

@gamtiq
Copy link

gamtiq commented Mar 6, 2012

Brian,

Some environments don’t have reasonable debuggers. ;-)
For example, I develop for devices where I can’t use any JS debugger so I have to utilize logging for bugs catching. Having proper toString implementation would be helpful in similar situations.

@briancavalier
Copy link
Member Author

That is certainly a great point. I've worked in a similar environment in the past (J2ME), and so I should know better than to assume everyone has access to a reasonable debugger :)

One thing I do when I need to resort to console.log or alert for debugging promises is:

function logPromise(promise, tag) {
    when(promise,
        function(value) {
            console.log("Promise(" + tag + ") resolved", value);
        },
        function(err) {
            console.error("Promise(" + tag + ") rejected", err);
        }
    );
}

And now that when/timeout is available, it's possible to log a message if a promise doesn't resolve after some reasonable amount of time, which can be some of the most difficult cases to track down. In the case of a timeout, the following will log an Error that indicates it timed out:

// If you're using node or AMD, you can load when/timeout and call it whatever you want, or
// if you're using script tags, load when/timeout, and use the when_timeout global

function logPromise(promise, tag) {

    // Setup a *new* promise that will timeout if the provided promise doesn't resolve or
    // reject within 5 seconds.
    // Note that this *does not* alter the original promise in any way, nor does it cause
    // the original promise to reject after 5 seconds.

    when_timeout(promise, 5000).then(
        function(value) {
            console.log("Promise(" + tag + ") resolved", value);
        },
        function(err) {
            console.error("Promise(" + tag + ") rejected", err);
        }
    );
}

That will cover cases where the promise is resolved or rejected explicitly, or doesn't resolve/reject within a particular amount of time. Is that helpful for you? Do you feel like a toString() would still provide some benefit? If so, do you think it should:

  1. return "[object Promise]", or
  2. return "[object Promise + indication of state: pending, resolved, rejected]", or
  3. return "[object Promise + indication of state plus resolution value or rejection reason]"
  4. Something else?

Thanks!

@gamtiq
Copy link

gamtiq commented Mar 6, 2012

Brian,

Thank you for hints and the code. Certainly, this is very helpful and can be used for logging and debugging.
But IMHO providing toString() to specific classes and objects is a good manner, especially, for library.
As for concrete implementation I would prefer the third variant.

@ghost ghost assigned briancavalier Apr 23, 2012
@briancavalier
Copy link
Member Author

There will be a way to enable a toString() method, in addition to a few other debug features, on promises, deferreds, and resolvers in the upcoming v1.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants