Hi all, Just wondering if this is any better. I've tried to implement what Martin has suggested best to my knowledge.
Any further feedback is appreciated. Thanks // Set up the app var express = require('express') , app = express() , fs = require('fs') , path = require('path'); // Define the path to the templates and set it to 'dir' var url = path.resolve('./templates/'); // Get all files from the template dir and send them to readFile() to be read var readDir = function(){ fs.readdir(url, function (err,files){ if (err) { console.log(err.message); return; } // for each of the files... files.forEach(function (file) { var filePath = path.join(url, file); // Get a files stats fs.stat(filePath, function (err, stat) { // If the file is not a directory, read it if (stat && stat.isFile()) { fs.readFile(filePath, 'utf8', function (err,data) { if (err) { console.log(err.message); return; } console.log('read this ' + data); }); } }); }); // Assuming the readdir() is complete, lets call templatesLoaded(); templatesLoaded(err); }); }; // when readdir() is complete, call templatesLoaded() var templatesLoaded = function (err) { // Check err and bail out if something bad happened. if (err) { console.log(err.message); return; } app.listen(3001); console.log('Listening on port 3000'); }; // Default route app.get('/', function(req, res){ res.send('Styleguide'); }); // Call readDir(); readDir(); On Monday, November 12, 2012 8:23:16 AM UTC+11, kinghokum wrote: > > Thanks Martin, I appreciate it. > I'll look into how to implement your suggestions. > > On Monday, November 12, 2012 5:42:39 AM UTC+11, Martin Cooper wrote: >> >> A few things: >> >> * You're loading all of the files each time you process a request. >> Assuming you want to load them on startup only, and make sure they're >> loaded before you start handling requests, you should do the loading in >> your mainline code, before you call listen. Something like: >> >> loadTemplates(function (err) { >> // Check err and bail out if something bad happened. >> >> // App Listen >> app.listen(3000); >> console.log('Listening on port 3000'); >> }); >> >> * The fs.readdir() and fs.readFile() functions are async. You should >> never be throwing from their callbacks, since you won't be around (in the >> call stack) to catch them. You need to be calling your own callback with >> any errors instead. >> >> * Since your readDir() function will complete asynchronously, its work is >> not yet complete when you call res.send(). (Addressing the first point >> above will address this, though.) >> >> * You should use the 'path' module to resolve the paths to your >> directories and files, instead of gluing them together as strings. >> >> Hope that helps. >> >> -- >> Martin Cooper >> >> >> On Fri, Nov 9, 2012 at 9:56 PM, Chris Buttery <but...@gmail.com> wrote: >> >>> Hi Everyone, I'm a long time reader, first time poster. >>> >>> I'm wondering if you guys could provide some feedback on this code I've >>> written, for my first attempt at an app. >>> >>> My intention is to read the contents of files from a specific directory. >>> So when you load this 'app' it iterates all the files in 'templates' and >>> sends them to my readFile() function to be read. >>> >>> This script works as I expected, but I'd like to see how some of your >>> more experienced developers would have attempted this. >>> >>> Thanks >>> >>> Chris >>> >>> // Set up the app >>> var express = require('express') >>> , app = express() >>> , fs = require('fs'); >>> >>> // Define the path to the templates and set it to 'dir' >>> app.set('templates', __dirname + '/templates'); >>> var dir = app.get('templates'); >>> >>> // Get all files from the template dir and >>> // send em to readFile() to be read >>> var readDir = function(){ >>> fs.readdir(dir, function (err,files){ >>> if(err) throw err; >>> >>> files.forEach(function(file) { >>> readFile(file); >>> }); >>> }); >>> }; >>> >>> // Read the files >>> var readFile = function (file) { >>> fs.readFile(dir+'/'+file, 'utf8', function (err,data) { >>> if(err) throw err; >>> console.log("read this " + data); >>> }); >>> }; >>> >>> // Default route >>> app.get('/', function(req, res){ >>> readDir(); >>> res.send('Styleguide'); >>> }); >>> >>> // App Listen >>> app.listen(3000); >>> console.log('Listening on port 3000'); >>> >>> -- >>> Job Board: http://jobs.nodejs.org/ >>> Posting guidelines: >>> https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines >>> You received this message because you are subscribed to the Google >>> Groups "nodejs" group. >>> To post to this group, send email to nod...@googlegroups.com >>> To unsubscribe from this group, send email to >>> nodejs+un...@googlegroups.com >>> For more options, visit this group at >>> http://groups.google.com/group/nodejs?hl=en?hl=en >>> >> >> -- Job Board: http://jobs.nodejs.org/ Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines You received this message because you are subscribed to the Google Groups "nodejs" group. To post to this group, send email to nodejs@googlegroups.com To unsubscribe from this group, send email to nodejs+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/nodejs?hl=en?hl=en