2
\$\begingroup\$

I am writing the Cloud function for Parse whose purpose is to update my database with a provided array of data objects. The main problem are the nested promises, which doesn't look right. Any help to refactor this code or any other critiques?

/** 
 * Updating multiple object arrays: 
 * Look for object with the same idKey and update with supplied data or create new if none is found
 * Expect Data {schema: <schema_name>, idKey: <name_of_id_key>, data: <object_array>}
 */

Parse.Cloud.define("updater", function(request, response) {
    var _ = require('underscore');

    var schema          = request.params.schema,
            idKey       = request.params.idKey,
            objectArray = request.params.data;

    var Class           = Parse.Object.extend(schema),
            parseObject, 
            id,
            query;

    _.each(objectArray, function(dataObject){
            id = dataObject[idKey];
            if (!id) return response.error("Supplied Object " + dataObject + " has no id Key '" + idKey + "' set!");

            // now query for the same idKey
            query           = new Parse.Query(Class);
            query.equalTo(idKey, id);
            query.find().then(
                function(array){
                    if(array) {
                        // found at least one object - take the first
                        parseObject = array[0];

                        // use the new dataObject to update parseObject
                        parseObject.set(dataObject);
                        parseObject.save().then(

                            /*
                             * should this go outside the promise?
                             */

                            function(object){
                                // success
                            },
                            function(error){
                                return response.error(error);
                            });                 
                    } else {
                        // nothing found - create new
                        parseObject = new Class(dataObject);
                        parseObject.save().then(
                            function(object){
                                // success
                            },
                            function(error){
                                return response.error(error);
                            });                 
                    }
                }
            );

            parseObject = new Class(dataObject);

            parseObject.save().then(
            function(object){
                // success
            },
            function(error){
                response.error(error);
            });
    });

    response.success("Success!");

});
\$\endgroup\$
2
  • \$\begingroup\$ This may just be me not knowing Parse's system, but the flow seems a little shaky: It looks like it'll call response.success right away, because it doesn't wait for all the save operations to finish. And if one of those do fail, it'll call response.error - except, as mentioned, I imagine the response object has long since been resolved successfully, and I imagine it can only be resolved once. And even if one save fails, the code keeps going with the rest of the objects. It all seems kinda suspect to me. \$\endgroup\$
    – Flambino
    Commented Apr 27, 2014 at 18:44
  • \$\begingroup\$ @Flambino Good point - these should be chained with .then - have to re-think it ... \$\endgroup\$ Commented Apr 28, 2014 at 6:22

2 Answers 2

2
\$\begingroup\$

I'd start with refactoring the promises functions and remove any code repetition, something like this:

...

function success(object){
    // success                          
}
function error(error){
    return response.error(error);
}
...

query.find().then(function(array){
    if(array) {
        // found at least one object - take the first
        parseObject = array[0];        
        parseObject.set(dataObject);        
     } else {
        // nothing found - create new
        parseObject = new Class(dataObject);
     }    
     parseObject.save().then(success,error); 
});
...
\$\endgroup\$
1
  • \$\begingroup\$ Well that still has .then inside another .then which I understand is to be avoided with promises. \$\endgroup\$ Commented Apr 28, 2014 at 6:20
1
\$\begingroup\$

Didn't know of this community until very recently. There is an alternative approach to composite promises proposed on StackOverflow: Writing Clean Code With Nested Promises.

There is also an implementation that describes an approach for a cleaner way to nested promises on Github: KATA - Chain of Promises influenced by the Rules and Chain of Responsibility design patterns.

In both of the above (Stack Overflow answer and GitHub link) the objective is to avoid having a large chain of nested promises.

In order to achieve that, the proposed implementation has been heavily influenced by the Rules and Chain of Command design patterns. That is:

  • Every promise adheres to a contract, which on the GitHub example is the BaseDummyPromise.
  • The BaseDummyPromise contract defines the isMatch method - which is used by the execution class (Main) in order to determine whether a promise on the chain needs to be executed or not.
  • All the promises are executed through the Main class and the result of a promise is the input for the next promise in the chain, allowing a rough implementation of the concepts of Chain of responsibility.

The end result would look something like the below, which looks much more neat than a large chain of .then().then().then()... that would be used in a composed promises methodology:

    /**
     * Executes a chain of pre-configured promises of type @see {@link BaseDummyPromise} until a promise on the chain fails.
     * The execution of the chain will stop when the first promise on the chain fails to be resolved.
     * @param chainOfPromises The chain of the promises to be executed.
     * @returns The successful response of each promise, or a failure response.
     */
    public async executeChainOfPromises(chainOfPromises: BaseDummyPromise<any>[]): Promise<any> {
        let promiseExecutionResult: any;

        for (let i = 0; i < chainOfPromises.length; i++) {
            if (chainOfPromises[i].isMatch(promiseExecutionResult)) {
                promiseExecutionResult = await chainOfPromises[i].execute(promiseExecutionResult);
            }
        }

        return promiseExecutionResult;
    }

Please note that the above example makes use of the async function, which is supported by most recent browsers, but older ones might not be supporting it. You can find the compatibility at async function browser compatibility.

The 2023 thing though would be to use observables instead of promises.

\$\endgroup\$
0

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.