3
\$\begingroup\$

Why this thing :

I work on a project where I need to call many asynchronous functions to get data, chain them, execute them if conditions are met,.. and I ended up having unreadable code.
At some point, I looked at promises library like Q.js but even with such tools my code was complicated, so I decided to write something which would make my cases of use simple to read.

Code + what it does :

The whole concept is to help someone generate the code for the asynchronous calls as a string and then call eval() to run it (the latter probably being a major case of evil programming).
At the moment, it has only two functions because I am not sure how good this idea is.

First function : chain
This functions chains two asynchronous calls, optionally with a condition on the result of the first asynchronous call.

pp_chain_function = function (fname,condition_string) {
var queue=this;
var prec_result = 'result_'+queue.queue_length;
queue.queue_string += !condition_string ? '.then(function(){return '+fname+'();})'
        :'.then(function('+prec_result+'){if('+prec_result+condition_string+'){return '+fname+'();}return $.when();})';
queue.length++;

return queue;
};

Second function : fail
Adds fail handler for the whole queue.

pp_fail_function = function(fname) {
    var queue = this;
    queue.queue_string += '.fail(function(error){return '+fname+'(error);})';
    return queue;
};

Global queue prototype :

queue_object = {
        pp_chain : pp_chain_function,
        pp_fail : pp_fail_function
        };  

Initializing the queue :

pp_begin = function(){
    var pp_queue = Object.create(queue_object); 
    pp_queue.queue_string='$.when()';
    pp_queue.queue_length=0;
    console.warn(pp_queue);
    return pp_queue;
};

Usage case :

var chain = pp_begin(); //init the chain
    chain.pp_chain('async_0').pp_chain('async_1');  //add two standard async functions
    if(condition two){chain.pp_chain('async_2','.length>0');} //if external conditions and async_1 gave a result with length>0, execute async_2
    chain.pp_fail('fail_handler');  //add fail handler
    console.warn(chain);  //replace with eval if you want to actually run it

I would love some feedback on that idea mainly, and on the code if applicable (even if there is very few).

EDIT : jsfiddle for up-to-date version

Concrete example :

Before :

 if (online_mode && i_s_r) { //if sync clear collections and reload them
            access.findAll()
                .then(function (result) {
                    if (result.length > 0) {
                        access.remove(result, {
                                push: false
                            })
                            .then(function () {
                                access.load()
                                    .then(function () {
                                        callback_function(i, true);
                                    });
                            });
                    } else {
                        access.load()
                            .then(function () {
                                callback_function(i, true);
                            });
                    };
                })
                .fail(function (errorObject) {
                    alert(errorObject);
                });
        } else {
            callback_function(i, true);

After :

var q0 = begin(); 
        if(online_mode && i_s_r){q0.chain('access.findAll()').chain('access.remove(result_1,{push: false})','.length>0').chain('access.load()').failure('alert');}
        q0.chain_sync('callback_function(i,true)');
        eval(q0.queue_string);
\$\endgroup\$
1
  • \$\begingroup\$ Do a search for: "javascript async await semantics," these tools are great for solving the problem of deeply nested callback functions. Eg: github.com/bjouhier/galaxy \$\endgroup\$
    – Ami
    Jul 17, 2014 at 13:48

1 Answer 1

5
\$\begingroup\$

Interesting question,

I find this code hard to read, I wonder what you consider unreadable code.

Some remarks on

pp_chain_function = function (fname,condition_string) {
var queue=this;
var prec_result = 'result_'+queue.queue_length;
queue.queue_string += !condition_string ? '.then(function(){return '+fname+'();})'
        :'.then(function('+prec_result+'){if('+prec_result+condition_string+'){return '+fname+'();}return $.when();})';
queue.length++;

return queue;
};
  • Use jsbeautifier.org, always
  • Prefixing with pp makes no good sense
  • Postfixing with _string makes little sense
  • prec_result <- What does prec stand for
  • var queue=this;, you only need to re-assign this if you work with closures
  • Consider a templating function for your string building
  • That long ternary statements looks horrendous, consider splitting it over multiple lines

Some remarks on

queue_object = {
    pp_chain : pp_chain_function,
    pp_fail : pp_fail_function
};  
  • You use queue_object as a prototype, consider changing the name to reflect, also, JavaScript ought to be written in lowerCamelCase: queue_object -> queueObject -> queuePrototype

Some remarks on

var chain = pp_begin(); //init the chain
    chain.pp_chain('async_0').pp_chain('async_1');  //add two standard async functions
    if(condition two){chain.pp_chain('async_2','.length>0');} //if external conditions and async_1 gave a result with length>0, execute async_2
    chain.pp_fail('fail_handler');  //add fail handler
    console.warn(chain);  //replace with eval if you want to actually run it
  • Indenting is messed up
  • You are putting conditions in strings..
  • I am fairly certain that either by making async_1 fail if length < 1 or putting your check inside async_2 you could replace this with

    $.when( async_0, async_1 )
      .then( async_2, fail_handler );
    

The biggest elephant in the room is of course eval, you loose proper stack-traces, debugging, tooling ( jshint, jsbeautifier ), and open yourself up to all kinds of pain. I would suggest that you post one of your unreadable scripts, perhaps we can show you how to use Q so that they become readable without resorting to .. this.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks for your reply; some questions/answers : - prec stands for precedent which is french for previous (mistake, I'll replace it with previous) - - Why shouldn't I put conditions in strings? - I can not modify the asynchronous functions' code, otherwise it would indeed be simpler =P - By templating fuction, do you mean I should store the template string somewhere with dummy args and use .replace()? - all the points I did not mention make sense to me and I'll try to make the corresponding changes \$\endgroup\$ Jul 17, 2014 at 4:38
  • \$\begingroup\$ +1 for "post one of your unreadable scripts." I would really like to see a before and after example. As it is I can't imagine this making anything more readable. \$\endgroup\$
    – Dagg
    Jul 17, 2014 at 6:48
  • 1
    \$\begingroup\$ Posted a case of use, well I don't know if it is more readable but it is shorter for sure =P Of course it could be improved further but I am still not quite sure how good the concept is.. \$\endgroup\$ Jul 17, 2014 at 10:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.