Skip to content
This repository has been archived by the owner on Dec 20, 2020. It is now read-only.

Function parameters anti-pattern #123

Open
csimi opened this issue Jun 28, 2016 · 8 comments
Open

Function parameters anti-pattern #123

csimi opened this issue Jun 28, 2016 · 8 comments

Comments

@csimi
Copy link

csimi commented Jun 28, 2016

Right now the parameters are passed to the handler functions like this:

function(xhr, response)

Isn't this a promise anti-pattern? I have to use intermediate functions to get the response instead of directly manipulating it like:

qwest.get('/someArrayResource')
         .then(_.first)
         .then(doWhatever);

Having the XHR object as the primary parameter in the function would be useful only in a few situations, where for example you care more about the status code instead of the response, which only happens rarely.

@pyrsmk
Copy link
Owner

pyrsmk commented Jun 30, 2016

You're right. response should be the first parameter, and xhr should be available with this keyword. I'll plan this for a future major release of qwest. Unless you really need this ASAP, I'll release it in the next months.

@phillip-haydon
Copy link

This will mess up es6 arrow functions.

this.products = [];

quest.get(....)
.then((xhr, response) => {
   this.products = response.products;
});

:/

@pyrsmk
Copy link
Owner

pyrsmk commented Jul 6, 2016

Hum, indeed... What would you suggest?

@phillip-haydon
Copy link

Personally I think the API is fine, but if it were to change, just swap the args around.

(response, xhr) effectively makes xhr optional, keeps the response the main thing the user wants/needs, doesn't mess with any scoping in ES5/6

Sometimes anti-patterns are necessary. (not that I consider this an anti-pattern to begin with)

@pyrsmk
Copy link
Owner

pyrsmk commented Jul 7, 2016

Indeed, It seems the best way to do it. I plan this change for a future major release then. I need to refactor qwest's code a bit, it's kinda messy ^^

@pyrsmk pyrsmk added the v5 label Jul 8, 2016
@okonomiyaki3000
Copy link

Sorry this question is only slightly related but I didn't find the answer in the docs.

If I have:

qwest.get(...)
    .then(function (response, xhr) {
        return "whatever";
    })
    .then(function (???) { ... });

The first then returns a promise that is resolved with "whatever", right? So, have I lost xhr at this point? Is there any way to get xhr in subsequent promises?

@pyrsmk
Copy link
Owner

pyrsmk commented Aug 19, 2016

You're right, at this point xhr is lost. But I can make some method to get it anyway... I should investigate this ^^

@csimi
Copy link
Author

csimi commented Aug 19, 2016

It's pretty easy to keep the xhr but you're getting into pyramid territory. Return a new promise chain.

qwest
    .get(...)
    .then(function (response, xhr) {
        return Promise
            .resolve(response)
            .then(function (???) {
                ...
            });
    });

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

No branches or pull requests

4 participants