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 <[email protected]> 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 [email protected]
>>> To unsubscribe from this group, send email to
>>> [email protected]
>>> 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 [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en
