When example

gallery when.js

{
   get_next: function(on_success, on_error, offset) { // Need to support on_success and on_error for legacy
      var deferred = when.defer();
      deferred.then(on_success, on_error);
      
      if (this.can_get_data()) {
        this.get_data({
          offset: offset
        }).then(function(e) { // Is this how I go about chaining promises ?
          deferred.resolve(e);
        }, function(e) {
          deferred.reject(e);
        });
      } else {
        deferred.reject("Not allowed to get data");
      }
      
      return deferred.promise;
    },
    get_data: function(overrides) {
      var self = this;
      var deferred = when.defer();
      
      var data_source_name = this.get_data_source_name();
      
      if (data_source_name && typeof data_source_name === 'string') {
        if (typeof this.kawr[data_source_name] == "function") {
          var params = this.get_data_source_parameters(overrides);
          
          if (params && typeof params === 'object') {
            
            this.pending_items = true;

            // this function returns a promise am I chaining it correctly ?
            this.kawr[data_source_name](params).then(
              function(e) {
                self.pending_items = false;
                deferred.resolve(e);
              }, function (e) {
                self.pending_items = false;
                deferred.reject(e);
              }
            );
          } else {
            deferred.reject("No parameters we returned");
          }
        } else {
          deferred.reject("Method '" + data_source_name + "' not defined on kawr";)
        }
      } else {
        deferred.reject("No data source name was defined");
      }
      
      return deferred.promise;
    }
}

Hey Raul,

What you have looks like it will work just fine. I think it can be simplified a bit--one thing to keep in mind is that not every method has to create it's own deferred, and many times you can simply pass an existing one back up the call stack.

Here's a couple quick and dirty attempts at refactoring the get_next() method to use that approach instead of creating a new deferred(although it still does in one case):

// This is a useful function to keep around
// It returns an already-rejected promise
function rejected(reason) {
    var deferred = when.defer();
    deferred.reject(reason);
    return deferred.promise;
}

// Here's a simplified version that I think accomplishes the same thing
get_next: function(on_success, on_error, offset) { // Need to support on_success and on_error for legacy
    var promise;

    promise = this.can_get_data()
        ? this.get_data({ offset: offset })
        : rejected("Not allowed to get data");

    // This is subtle: The promise returned here will resolve/reject
    // with the return value of on_success/on_error (unless they return undefined),
    // which may not be what you want, but if it doesn't matter, then this
    // will work just fine.
    // See below for alternative
    return when(promise, on_success, on_error);

    // Alternative
    // The promise returned here will resolve with the resolution value
    // from can_get_data.
    //when(promise, on_success, on_error);
    //return promise;
},

Depending on the situation, you may also want to take a look at when.chain(), which can be useful when you absolutely, positively need to chain two promises together.

If you could do away with, or have the caller handle/register the on_success and on_error callbacks with a returned promise, you could end up with something very compact, like the following:

// Here's a super-simplified version if you didn't have to worry about the
// legacy on_success and on_error callbacks, and could just return the
// promise
get_next: function(offset) {
    return this.can_get_data()
        ? this.get_data({ offset: offset })
        : rejected("Not allowed to get data");
},

Awesome! Thanks for the tips. It all makes a lot more sense now. I bit the bullet factored out the legacy on_success and on_error callbacks which also led to cleaner code all around. Does when.js support a .done() method ?
If you look at the callback I have

this.kawrdata_source_name.then(
function(e) {
self.pending_items = false;
deferred.resolve(e);
}, function (e) {
self.pending_items = false;
deferred.reject(e);
}


I set self.pending_items = false; on both the success case and error case. I was wondering if there was a method I could use that would get called on both an error and success.

Thanks,
Raul

Right now, there's no shortcut for a done or finally type handler, but it's something I could consider adding. If you run into more cases where you really need it, let me know.

It's basically equivalent to promise.then(doDoneStuff, doDoneStuff) or when(promise, doDoneStuff, doDoneStuff), so off the top of my head, here's a version of get_data() that does something along those lines, and passes the kawr promise up the stack:

get_data: function(overrides) {
    var self = this;
    var result;

    function reset_pending_items() {
        self.pending_items = false;
    }

    var data_source_name = this.get_data_source_name();

    if (data_source_name && typeof data_source_name === 'string') {
        if (typeof this.kawr[data_source_name] == "function") {
            var params = this.get_data_source_parameters(overrides);

            if (params && typeof params === 'object') {

                this.pending_items = true;

                // this function returns a promise am I chaining it correctly ?
                result = this.kawr[data_source_name](params)
                    .then(reset_pending_items, reset_pending_items);

            } else {

                result = rejected("No parameters we returned");
            }
        } else {
            result = rejected("Method '" + data_source_name + "' not defined on kawr";)
        }
    } else {
        result = rejected("No data source name was defined");
    }

    return result;
}

Ah that makes things even more clear.
I guess I didn't completely read the comment you added in the last example

// This is subtle: The promise returned here will resolve/reject
// with the return value of on_success/on_error (unless they return undefined),

I was thinking I had to refactor it like so
when(result, reset_pending_items, reset_pending_items)

So I can think of all the .then() chains as mutators unless they return undefined. I guess I can go through and clean up my code a bit more in other places.

Thanks for the help :)

Raul

Thinking of them as mutators is a great way to put it--exactly right. The key is that promise and the promise returned by when or .then are different promises, and thus can have different resolution values.

Whether or not to mutate when undefined is returned is a bit of a gray area, since it's impossible to tell whether the callback intentionally returned undefined, or it simply didn't return a value at all. The Promises/A spec isn't fully clear on it, so I went with not mutating since that seems more intuitive to me ... I mean, how many times have you intentionally written return undefined?!?

BTW, not sure if it was obvious or not, but when(promise, callback, errback) and promise.then(callback, errback) are exactly the same. The advantage of when() is that it can handle situations where you don't know whether you have a promise or a value, e.g.: when(mightBeAPromiseOrARegularValue, callback, errback). It also tends to compress/minify slightly better than .then() :)