Skip to main content
Code Review

Return to Answer

replaced http://softwareengineering.stackexchange.com/ with https://softwareengineering.stackexchange.com/
Source Link

Here's something that bothers me about this code. You have all the flexibility of Express, yet you've structured you're project like an Asp.Net project.

/
|--index.js
|
|--Views/
| |--todo.hbs
|
|--Controllers/
| |--todo.js
|
|--Repositories/
| |--todoRepository.js
|
|--Middleware/
| |--...
| 
|--Test/
 |--...

You, yourself, wrote an answer on Software Engineering talking about how this is cargo cult programming wrote an answer on Software Engineering talking about how this is cargo cult programming.

[A project's structure] should tell me that it's a store front, or a time off request system, or whatever. The high level structure and architecture should tell us about what this thing is, not how it was implemented.

It would be much nicer to structure the project so that it's immediately evident what this software does.

/
|--index.js
|
|--Todo/
 |--controller.js
 |--repository.js
 |--index.hbs

This structure removes the awkward and inconsistent naming you had previously, giving you import statements that look like this.

var todoDb = require('../Todo/repository');

and this

var root = require('./Root/controller'),
 todo = require('./Todo/controller'); 

instead of this

var todoDb = require('../repositories/todoRepository');

and this

var root = require('./controllers/root'),
 todo = require('./controllers/todo'); 

Grouping your objects by functionality like this both makes it really stupid easy to name things and grants you this great kind of "namespacing". Anytime you need to modify the "todo" functionality in any way, you'll know exactly where to look for the file.

There is a small downside to this however, you'll need to explicitly tell Express to check these new directories for views.

app.set('view engine', 'hbs');
app.set('views', [__dirname + '/todo', __dirname + '/views']);

Here's something that bothers me about this code. You have all the flexibility of Express, yet you've structured you're project like an Asp.Net project.

/
|--index.js
|
|--Views/
| |--todo.hbs
|
|--Controllers/
| |--todo.js
|
|--Repositories/
| |--todoRepository.js
|
|--Middleware/
| |--...
| 
|--Test/
 |--...

You, yourself, wrote an answer on Software Engineering talking about how this is cargo cult programming.

[A project's structure] should tell me that it's a store front, or a time off request system, or whatever. The high level structure and architecture should tell us about what this thing is, not how it was implemented.

It would be much nicer to structure the project so that it's immediately evident what this software does.

/
|--index.js
|
|--Todo/
 |--controller.js
 |--repository.js
 |--index.hbs

This structure removes the awkward and inconsistent naming you had previously, giving you import statements that look like this.

var todoDb = require('../Todo/repository');

and this

var root = require('./Root/controller'),
 todo = require('./Todo/controller'); 

instead of this

var todoDb = require('../repositories/todoRepository');

and this

var root = require('./controllers/root'),
 todo = require('./controllers/todo'); 

Grouping your objects by functionality like this both makes it really stupid easy to name things and grants you this great kind of "namespacing". Anytime you need to modify the "todo" functionality in any way, you'll know exactly where to look for the file.

There is a small downside to this however, you'll need to explicitly tell Express to check these new directories for views.

app.set('view engine', 'hbs');
app.set('views', [__dirname + '/todo', __dirname + '/views']);

Here's something that bothers me about this code. You have all the flexibility of Express, yet you've structured you're project like an Asp.Net project.

/
|--index.js
|
|--Views/
| |--todo.hbs
|
|--Controllers/
| |--todo.js
|
|--Repositories/
| |--todoRepository.js
|
|--Middleware/
| |--...
| 
|--Test/
 |--...

You, yourself, wrote an answer on Software Engineering talking about how this is cargo cult programming.

[A project's structure] should tell me that it's a store front, or a time off request system, or whatever. The high level structure and architecture should tell us about what this thing is, not how it was implemented.

It would be much nicer to structure the project so that it's immediately evident what this software does.

/
|--index.js
|
|--Todo/
 |--controller.js
 |--repository.js
 |--index.hbs

This structure removes the awkward and inconsistent naming you had previously, giving you import statements that look like this.

var todoDb = require('../Todo/repository');

and this

var root = require('./Root/controller'),
 todo = require('./Todo/controller'); 

instead of this

var todoDb = require('../repositories/todoRepository');

and this

var root = require('./controllers/root'),
 todo = require('./controllers/todo'); 

Grouping your objects by functionality like this both makes it really stupid easy to name things and grants you this great kind of "namespacing". Anytime you need to modify the "todo" functionality in any way, you'll know exactly where to look for the file.

There is a small downside to this however, you'll need to explicitly tell Express to check these new directories for views.

app.set('view engine', 'hbs');
app.set('views', [__dirname + '/todo', __dirname + '/views']);
Source Link
RubberDuck
  • 31.2k
  • 6
  • 74
  • 176

Here's something that bothers me about this code. You have all the flexibility of Express, yet you've structured you're project like an Asp.Net project.

/
|--index.js
|
|--Views/
| |--todo.hbs
|
|--Controllers/
| |--todo.js
|
|--Repositories/
| |--todoRepository.js
|
|--Middleware/
| |--...
| 
|--Test/
 |--...

You, yourself, wrote an answer on Software Engineering talking about how this is cargo cult programming.

[A project's structure] should tell me that it's a store front, or a time off request system, or whatever. The high level structure and architecture should tell us about what this thing is, not how it was implemented.

It would be much nicer to structure the project so that it's immediately evident what this software does.

/
|--index.js
|
|--Todo/
 |--controller.js
 |--repository.js
 |--index.hbs

This structure removes the awkward and inconsistent naming you had previously, giving you import statements that look like this.

var todoDb = require('../Todo/repository');

and this

var root = require('./Root/controller'),
 todo = require('./Todo/controller'); 

instead of this

var todoDb = require('../repositories/todoRepository');

and this

var root = require('./controllers/root'),
 todo = require('./controllers/todo'); 

Grouping your objects by functionality like this both makes it really stupid easy to name things and grants you this great kind of "namespacing". Anytime you need to modify the "todo" functionality in any way, you'll know exactly where to look for the file.

There is a small downside to this however, you'll need to explicitly tell Express to check these new directories for views.

app.set('view engine', 'hbs');
app.set('views', [__dirname + '/todo', __dirname + '/views']);
default

AltStyle によって変換されたページ (->オリジナル) /