0

From what I've read, nesting functions in javascript causes extra declarations / destructions which can be avoided by using "static functions" or even a Singleton implementation. Also "new" does the same thing where two instances of the function or objects are also independent copies.

Is this true? If so, what can I do to have the same functionality as with nested functions and with "new". This is for a game where the server is in nodejs / javascript. I've reached about 8 levels of nested functions and am starting to worry.

Example:

DB.cityUpdateUpkeep = function( cuid )
{
/**  @type {Array} */
var buildings = null;

DB.cityGet( cuid, function( error, city )
{
    if( error )
    {
        console.log( "Couldn't get city" );
    }
    else
    {
        DB.iBuildings.find( {cuid:cuid}, function( error, cursor )
        {
            if( error )
            {
                console.log( "-error:" );
                console.log( error );
            }
            else
            {
                cursor.toArray( function( error, response )
                {
                    if( error )
                    {
                        console.log( "-error:" );
                        console.log( error );
                    }
                    else
                    {
                        console.log( "-the response:" );
                        console.log( response );
                        buildings = response;

                        var income  = city.resources.income;
                        var storage = city.resources.storage;
                        var stored  = city.resources.stored;

                        for( var buildingID in buildings )
                        {
                            var building = buildings[ buildingID ];
                            var blueprint = DB.bBuildings[ building.buid ];

                            if( blueprint.resources.income )
                            {
                                income = Utils.sumObjects( income, blueprint.resources.income );
                            }

                            if( blueprint.resources.storage )
                            {
                                storage = Utils.sumObjects( storage, blueprint.resources.storage );
                            }

                            if( blueprint.resources.stored )
                            {
                                stored = Utils.sumObjects( stored, blueprint.resources.stored );
                            }
                        }

                        console.log( "cuid: " + city._id + " income: " + income + " storage " + storage + " stored " + stored );
                    }
                });
            }
        });
    }
});
};
Improve this question
2
  • 3
    How are you nesting them? What functions are these? 8 levels? seriously? sounds like callbacks rather than nested functions. Commented May 14, 2013 at 11:25
  • Could you provide an example? The question as it is now it's too generic and would be addressed by pointing you to articles talking about naming functions, using promises, using events, ... without really helping you with your problem. Commented May 14, 2013 at 11:27

3 Answers 3

Reset to default
1

Take a look at Q for ways to flatten callbacks (the code would be little nicer). For your specific example, I would prefer to refactor using a number of methods:

  1. Return methods early when err (saves you from having to nest else's)
  2. Execute asynchronous calls concurrently when they're not dependent (notice DB.cityGet and DB.iBuildings.find - makes your code run faster as well)
  3. creating functions and referencing outside the nest (ex. checkComplete)

My refactor below:

  DB.cityUpdateUpkeep = function( cuid ){
    /**  @type {Array} */
    var buildings = null;
    var city = null;
    var checkComplete = function(){
        if (!city || !builings){
            return;
        }   
        var income  = city.resources.income;
        var storage = city.resources.storage;
        var stored  = city.resources.stored;

        for( var buildingID in buildings ){
            var building = buildings[ buildingID ];
            var blueprint = DB.bBuildings[ building.buid ];

            if( blueprint.resources.income ){
                income = Utils.sumObjects( income, blueprint.resources.income );
            }   

            if( blueprint.resources.storage ){
                storage = Utils.sumObjects( storage, blueprint.resources.storage );
            }   

            if( blueprint.resources.stored ){
                stored = Utils.sumObjects( stored, blueprint.resources.stored );
            }   
        }   

        console.log( "cuid: " + city._id + " income: " + income + " storage " + storage + " stored " + stored );

    }   
    DB.cityGet(cuid, function(err, response){
        if (err){
            console.log("Couldn't get city");
            return;
        }   
        city = response;
        checkComplete();
    }); 
    DB.iBuildings.find({cuid:cuid}, function(err, cursor){
        if (err){
            console.log(err);
            return;
        }   
        cursor.toArray(function(err, response){
            if (err){
                console.log(err)
                return;
            }   
            buildings = response;
            checkComplete(); 
        }); 
    }); 
});
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

It does, indeed look better. You still create checkComplete and the handlers each time you call cityUpdateUpkeep. Still, +1 for the paralleled calls. I had trouble figuring out the scope, but now that I see it, I feel silly how simple it is :))
oops you are right about the checkComplete. Simply move that outside the function. The handlers should get cleaned up in modern javascript engines. In fact, from what I understand, the checkComplete also won't be reallocated each time - but best to be safe.
0

One solution is to store the functions instead of having anonymous callbacks. It's easier to reuse them as well.

DB.cityUpdateUpkeep = function (cuid) {
    var findHandler = function () { ... };

    DB.cityGet(cuid, function (error, city) {
        ...
        DB.iBuildings.find({cuid:cuid}, findHandler);
        ...
    });
};
Improve this answer

1 Comment

Note how I use "city" from top-level all the way to bottom level. I would have to heavily parametrize all these functions which might overcomplicate things. Just consider me adding a parameter in top-level which I need in the bottom; I would have to pass that value in the entire chain :| Just how bad are these nested functions, anyway?
0

I would suggest that you pick a control flow library (async is the most popular one) instead of having a bunch of callbacks, you can pick waterfall

async.parallel([
    function(callback){
        setTimeout(function(){
            callback(null, 'one');
        }, 200);
    },
    function(callback){
        setTimeout(function(){
            callback(null, 'two');
        }, 100);
    }
],
// optional callback
function(err, results){
    // the results array will equal ['one','two'] even though
    // the second function had a shorter timeout.
});

(code is borrowed from the main website of async). So if you want to share a variable between those nested functions, you can define it in DB.cityUpdateUpkeep which is the top-level, and other nested functions can use it. However, be aware that if you modify the variable at any time, the function that runs later will be affected.

And, how bad are these nested functions? Nothing, it is the nodejs nature (asynchronous programming model), you have to live with it. You will have to re-arrange your code for easy maintenance, though

Improve this answer

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.