CODEWITHSUNDEEP
node.js
Justin808
Justin808

Reputation: 21522

New to node.js and I'm not sure if I'm doing this correct

So, the code works, but the indent level is insane... With all the callbacks in node, how exactly should I be coding?

"use strict";

var crypto = require('crypto'),
    fs     = require('fs'),
    mmm    = require('mmmagic'),
    Magic  = require('mmmagic').Magic,
    path   = require('path');

console.log('Init controller: ' + path.basename(__filename));

exports.help = function () {
    var help;

    help  = "POST http://server/images\n";
    help += "    Upload image for storage.\n";
    help += "    <image> - The image file to upload\n";
    help += "    <title> - The title of the image, no more than 50 characters\n";
    help += "    <desc>  - The description of the image, no more than 1024 characters\n";

    return help;
}

exports.post = function (req, res) {
    var image = req.files.image;
    if (typeof(image) == 'undefined') {
        res.status(400).send("{error:'Upload error'}");
        return;
    }

    var magic = new Magic(mmm.MAGIC_MIME_TYPE);
    magic.detectFile(image.path, function(err, result) {
        if (err) {
            res.status(400).send("{error:'Upload mime error'}");
        } else {
            var mime = result.toLowerCase().split('/');

            if (mime[0] != 'image') {
                res.status(400).send("{error:'Upload not an image', mime: '" + result + "'}");
            } else {
                // Read the image file
                fs.readFile(image.path, function (err, data) {
                    if (err) {
                        res.status(400).send("{error:'Upload read error'}");
                    } else {
                        var hash = crypto.createHash('md5').update(data).digest("hex");
                        req.app.models.image.count({'hash': hash}, function (err, count) {
                            if (err) {
                                res.status(400).send("{error:'ORM Error: '" + JSON.stringify(err) + "'}");
                            } else {
                                if (count > 0) {
                                    res.status(400).send("{error:'Image already exists'}");
                                } else {
                                    var hash = crypto.createHash('md5').update(data).digest("hex");
                                    var newPath = path.join(req.app.tempDir, hash);
                                    fs.writeFile(newPath, data, function (err) {
                                        if (err) {
                                            res.status(400).send("{error:'Upload write error'}");
                                        } else {
                                            // Save the image
                                            req.app.models.image.create([{
                                                'hash'      : hash,
                                                'mime'      : mime,
                                                title       : '',
                                                description : ''
                                            }], function(err, images) {
                                                if (err) {
                                                    fs.unlink(newPath);
                                                    res.status(400).send("{error:'" + err.message + "'}");
                                                } else {
                                                    res.status(200).send("{id:'" + images[0].id + "}");
                                                }
                                            });
                                        }
                                    });
                                }
                            }
                        });
                    }
                });
            }
        }
    });
}

Upvotes: 2

Views: 256

Answers (6)

Peter Lyons
Peter Lyons

Reputation: 146084

http://callbackhell.com/ has a guide to adjusting to asynchronous programming.

Some follow-ups from the comments:

  • There are projects such as Node Fibers and Iced CoffeeScript if making asynchronous code look like top-down synchronous code appeals to you and the being misled by that illusion doesn't make you very nervous. However, I strongly recommend working with regular javascript and async programming with callbacks (not promises) until the light bulb goes on before exploring solutions to a "problem" you don't really understand.
  • Asynchronous code is a bunch of bits of code that execute when the OS is done with I/O. That's why it doesn't read top-down - because it doesn't execute top-down. It executes whenever the IO is done which is why it scales the way it does.
  • Don't be misled by beginner snippets. Go look at code by the masters like TJ Holowaychuk before deciding that asynchronous code is unreadable. Callback hell is a beginner phenomenon. It is extremely prevalent. Almost everyone goes through it as a phase, but it's not particularly difficult to get beyond, especially given the alternative of mastering multithreaded locking and semaphores and the debugging thereof. Async code that reads well is often just straight-up better-designed code all around. But yes, it lacks the top-down orderliness of synchronous code and that means more jumping around in the source.

Upvotes: 5

Dmitry Manannikov
Dmitry Manannikov

Reputation: 1154

Instead of promises I prefer to use node-sync lib. Because it allow one great thing: you don't must wrap async libs functions for promises, you can just use it with special syntax. Like this:

var result = thirdPartyLibAsyncFunction.sync(null, 2, 3);

It work with your own code the same way.

Upvotes: 0

Krasimir
Krasimir

Reputation: 13539

Come one guys. Do you really think that you need a library for that. You can handle it with pure javascript. Here is the code rewritten:

var response = null,
    request = null;
var fileDetected = function(err, result) {
    if (err) {
        response.status(400).send("{error:'Upload mime error'}");
    } else {
        var mime = result.toLowerCase().split('/');
        if (mime[0] != 'image') {
            response.status(400).send("{error:'Upload not an image', mime: '" + result + "'}");
        } else {
            // Read the image file
            fs.readFile(image.path, onReadFile);
        }
    }
}
var onReadFile = function(err, data) {
    if (err) {
        response.status(400).send("{error:'Upload read error'}");
    } else {
        var hash = crypto.createHash('md5').update(data).digest("hex");
        request.app.models.image.count({'hash': hash}, onImageCount);
    }
}
var onImageCount = function(err, count) {
    if (err) {
        response.status(400).send("{error:'ORM Error: '" + JSON.stringify(err) + "'}");
    } else {
        if (count > 0) {
            response.status(400).send("{error:'Image already exists'}");
        } else {
            var hash = crypto.createHash('md5').update(data).digest("hex");
            var newPath = path.join(request.app.tempDir, hash);
            fs.writeFile(newPath, data, onFileWrite);
        }
    }
}
var onFileWrite = function(err) {
    if (err) {
        response.status(400).send("{error:'Upload write error'}");
    } else {
        // Save the image
        request.app.models.image.create([{
            'hash'      : hash,
            'mime'      : mime,
            title       : '',
            description : ''
        }], function(err, images) {
            if (err) {
                fs.unlink(newPath);
                response.status(400).send("{error:'" + err.message + "'}");
            } else {
                response.status(200).send("{id:'" + images[0].id + "}");
            }
        });
    }
}

exports.post = function (req, res) {
    request = req;
    response = res;
    var image = request.files.image;
    if (typeof(image) == 'undefined') {
        response.status(400).send("{error:'Upload error'}");
        return;
    }
    var magic = new Magic(mmm.MAGIC_MIME_TYPE);
    magic.detectFile(image.path, fileDetected);
}

The good part is that by separating everything in different functions you actually split your logic into blocks. The name of the newly created function speaks about the purpose of the block.

Upvotes: 1

bguiz
bguiz

Reputation: 28587

<shamelessPlug>

I went through the process of callback spaghetti --> promise spaghetti --> wrote my own promise management library

That library is available here

... and this describes the above process in a bit more detail

</shamelessPlug>

In any case, the general answer is yes - you will eventually wind up with a mess of callbacks. To stop that from happening, you are best served by a callback management system or promises.

Which method is the best? It really is up to you, and what you prefer.

Upvotes: 2

cababunga
cababunga

Reputation: 3114

I found that concept of promises helps here a lot. There are many implementations of promises in JavaScript, but idea is same. You can start from this one:

https://github.com/kriszyp/node-promise

Upvotes: 1

Amadan
Amadan

Reputation: 198408

If you need to comment that something is "load the image" or "save the image", extracting those into separate functions would help the readability, decrease the indent level, and remove the need for comments.

I.e. instead of writing

// ... 
      petAHorse(function(horsesResponse) {
        // ...
      })
// ...

you can write

function horsePettedHandler(horsesResponse) {
  // ...
}

// ... 
      petAHorse(horsePettedHandler);
// ...

Upvotes: 0

Related Questions