0

My question is because I believe that maybe there is a better way to design my code when I need to do sync tasks in JS and wait for the response.

I'm working on Node and I need to save some data (I have a function that do it), but I need to perform some checks before (vs the database) that are async calls also.

I will simplify code and examples:

function save(req, res) {
    var id = req.param('id');
    var name = req.param('name');

    if (id) {
        // Here come the checks, e.g., there is no limit in number of items on DB
        mymodule.getConfig("maxItems", function(data) {
            if (data.success) {
                // max = 0 means unlimited items
                if (data.maxItems == 0) {
                    // Just continue, all fine

                } else {
                    mymodule.countItems(function(data) {
                        if (data.success) {
                            if (data.count >= maxItems) {
                                res.render('myPage', {error: 'You have reached the maximun items'});
                            }
                        } else {
                            res.render('myPage', {error: 'Error in DB'});
                        }
                    });
                }

            } else {
                res.render('myPage', {error: 'Error in DB'});
            }
        });

        // Save part (common code)
        mymodule.save({
            'id': id,
            'name': name
        }, function(data) {
            if (data.success) {
                res.render('myPage');
            } else {
                res.render('myPage', {error: 'Error while saving in DB'});
            }
        });

    } else {
        res.render('myPage', {error: 'Id is null'});
    }
}

Obviously this will not work because checks will be called async and save will be called always, instead of the redirection when the checks fail.

The right way is to "duplicate" the save part in the right place, like this:

function save(req, res) {
    var id = req.param('id');
    var name = req.param('name');

    if (id) {
        // Here come the checks, e.g., there is no limit in number of items on DB
        mymodule.getConfig("maxItems", function(data) {
            if (data.success) {
                // max = 0 means unlimited items
                if (data.maxItems == 0) {
                    // Save part---------------------------
                    mymodule.save({
                        'id': id,
                        'name': name
                    }, function(data) {
                        if (data.success) {
                            res.render('myPage');
                        } else {
                            res.render('myPage', {error: 'Error while saving in DB'});
                        }
                    });

                } else {
                    mymodule.countItems(function(data) {
                        if (data.success) {
                            if (data.count >= maxItems) {
                                res.render('myPage', {error: 'You have reached the maximun items'});
                            } else {
                                // Save part-------------------------
                                mymodule.save({
                                    'id': id
                                    'name': name
                                }, function(data) {
                                    if (data.success) {
                                        res.render('myPage');
                                    } else {
                                        res.render('myPage', {error: 'Error while saving in DB'});
                                    }
                                });
                            }
                        } else {
                            res.render('myPage', {error: 'Error in DB'});
                        }
                    });
                }

            } else {
                res.render('myPage', {error: 'Error in DB'});
            }
        });

    } else {
        res.render('myPage', {error: 'Id is null'});
    }
}

This works but is a wrong way to code, duplicating the same code. To respect the DRY principle, currently I'm doing a function with a callback with the save part, and calling that function in each place, like this:

function saveData(id, name, function(onComplete) {
    mymodule.save({
        'id': id,
        'name': name
    }, function(data) {
        if (data.success) {
            onComplete('');
        } else {
            onComplete('Error while saving in DB');
        }
    });
});

function save(req, res) {
    var id = req.param('id');
    var name = req.param('name');

    if (id) {
        // Here come the checks, e.g., there is no limit in number of items on DB
        mymodule.getConfig("maxItems", function(data) {
            if (data.success) {
                // max = 0 means unlimited items
                if (data.maxItems == 0) {
                    // Save part---------------------
                    saveData(id, name, function(cb) {
                        res.render('myPage', {error: cb});
                    });
                } else {
                    mymodule.countItems(function(data) {
                        if (data.success) {
                            if (data.count >= maxItems) {
                                res.render('myPage', {error: 'You have reached the maximun items'});
                            } else {
                                // Save part---------------------
                                saveData(id, name, function(cb) {
                                    res.render('myPage', {error: cb});
                                });
                            }
                        } else {
                            res.render('myPage', {error: 'Error in DB'});
                        }
                    });
                }

            } else {
                res.render('myPage', {error: 'Error in DB'});
            }
        });

    } else {
        res.render('myPage', {error: 'Id is null'});
    }
}

I'm searching for a better way to do this because I'm still calling two (or more times if needed) the saveData function. The ideal flow was the first one, but due to the javascript async nature, I cannot make a "sequential" flow with "returns" with res calls for checkings, and a unique save call at the end if all worked fine.

Is there another simpler way to accomplish this?

  • 4
    Have you considered using promises? While it wouldn't completely change the flow, it would allow you to write code in a far simpler way. – Neil May 6 '15 at 13:33
  • I take a look on "it" one year ago and found it some "strange", but this BlueBird project seems to be easier than other promise modules I saw. I will study it carefully. Thanks. – Eagle May 6 '15 at 14:30
  • 1
    It's worth learning in my opinion. One of the reasons why you'd use promises is to avoid this type of "callback hell", which, allow me to say, seems to be your case. – Neil May 6 '15 at 14:33
  • I agree. I will use promises from now on. I was avoiding it to use "pure" javascript instead of "strange magic", but there is one point that the "cb hell" is so big to ignore it. – Eagle May 6 '15 at 20:49
  • I went through the exact same thing. It isn't worth it at least in this case imho. – Neil May 7 '15 at 7:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.