-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Feedback on Project 1: MVC with Node.js and Express
Issue #1: No start script in Package.json
You specified in your Readme that users can start the server by running npm start, but you didn't define the start script in your package.json. Here's an example:
"scripts": {
"start": "node index.js",
"watch": "nodemon index.js"
}Issue #2: No .gitignore file
Include a .gitignore file to exclude files you don't want to push to your remote repository, for example, .hintrc, .vscode, node_modules, .env, are not expected to be pushed along with your code.
Issue #3: Create an .env_example file
To ensure secure handling of sensitive information, it is recommended not to push the .env file containing your environment variables to your remote repository. Instead, we follow the best practice of creating an .env_example file. This file serves as a template and should be included in the root directory of your project, replacing the .env file.
Example .env_example file:
PORT=your-port-number
DB_HOST=your_database_host
DB_USERNAME=your_database_username
DB_PASSWORD=your_database_password
DB_DBNAME=your_database_nameIssue #4: Improper naming of the database file
In the part database/index.js, it is good practice to not name your database file "index.js" to avoid conflicting with the actual index.js, as the name "index" is a reserved name for identifying the starting point of your application. So we give it that respect, even though the current database/index.js works fine.
For example, the directory where the database configuration file is located is usually called "config" and the file name "database." Hence, instead of:
project1/database/index.js
You should have:
config/database.js or config/db.js
Issue #5: No views
Firstly, your project has no views (V), which is the front end of your application. You can use popular template engines like EJS, Handlebars, or even HTML to create your views that can be rendered by the functions defined in the Controller (controller/users.js) instead of rendering plain text with res.send() function.
Secondly, the controller/users.js has the correct code because in the MVC pattern, the Controller component handles the logic for handling HTTP requests, interacting with the Model (database or data access layer), and determining the appropriate View to render.
In your project, you defined several functions (createUser, getUsers, getUser, deleteUser, and updateUser) that correspond to different routes and their associated logic. These functions are responsible for handling the HTTP requests and responses, manipulating the users' data, and sending appropriate responses back to the client. In other words, the Controller receives the requests, communicates with the Model (which in your case is the users array), performs necessary operations, and returns the appropriate responses.
The Router component (router/users.js) handles the routing of incoming HTTP requests to the appropriate Controllers. It maps the routes to their corresponding functions in the Controllers, allowing the application to determine the appropriate logic to handle each request.
Thirdly, you should have implemented this task using a database as it makes more sense in the context of MVC.
To summarize the flow in an Express application using MVC:
- The client makes an HTTP request to a specific endpoint handled by a Controller (route handler) in Express.
- The Controller extracts any relevant data from the request and interacts with the Model (database) to fetch or update data.
- The Controller renders the appropriate View, passing the retrieved data as variables.
- The View is rendered by the server, incorporating the dynamic data provided by the Controller.
- The server sends the rendered View (usually HTML) as a response back to the client's browser.
By separating concerns into distinct components, the MVC pattern promotes code organization, maintainability, and reusability. It helps manage complexity in larger applications by providing a clear separation between data, business logic, and presentation layers.
Issue #6: Code in your index.js (main entry point)
You don't need to install bodyparser, which is a dependency for parsing JSON data. This is because the new version of Express comes along with a JSON method for parsing JSON. Hence, installing bodyparser is not necessary.
You can replace the app.use(bodyParser.json()) in line 13 with app.use(express.json()) without installing bodyparser.
Note: The express.json() was added to Express from version 4.16.0 to the latest. Your project is using Express version 4.18.2, which is the latest, so there's no need for bodyparser.
Feedback on Project 2: Data Import/Export
The steps provided to get started do not require steps 3 and step 4 because you have already cloned a Node.js project, which automatically creates the folder, and step 4 is unnecessary because it's already a Node.js project with a package.json included.
Issue #1: No dev or watch script set
It is good practice to set your dev command in the scripts object of your package.json file so you can run npm run dev, which starts your Express server instead of node index.js. Example:
"scripts": {
"start": "node index.js",
"watch": "nodemon index.js"
}Issue #2: No database file for testing
Always export the database as an SQL file and include it in your project if you want others to test it.
Issue #3: Scalability mindset
It is not compulsory that all your routes, database config, and middleware be inside your main entry point. You can define your database config in a separate file and export your function as a module. The same goes for the rest configuration and middleware.
Issue #7: Incomplete task
Your project is incomplete because users can only upload their CSV file to the database but cannot generate CSV, JSON, or Excel files from the stored data (CSV). If you want to perform some task, you can search for the dependency that will help perform that task, including the documentation on https://www.npmjs.com.
Remarkable pointers:
- Great Documentation on your Readme
- Very efficient and neat code
- Good use of .env file
Questions:
- Explain the purpose of these 2 lines of code in your main entry point (index.js)?
app.use(express.urlencoded({extended: false}));
app.use(express.json());-
Explain the flow in your
controller/usersController.js. -
What would you do if you decide to use
importandexportkeywords in place ofrequire? -
Explain updateUser function and what is the '?'