Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job setting up the HTML form—it correctly captures and sends the data as required. However, some changes are needed in src/createServer.js to fix a couple of critical issues that prevent the application from working correctly.
First, the server's routing logic currently tries to send multiple responses for a single request, which causes the application to crash. You can resolve this by using an if...else if...else structure to ensure only one response block is executed for each request.
Second, the core functionality for saving data is not working as intended. With every new submission, the db/expense.json file is overwritten, losing all previous entries. To fix this, you need to read the existing file content, parse it into an array, add the new expense object to that array, and then write the entire updated array back to the file.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
src/createServer.js
Outdated
| if (url !== '/submit-expense') { | ||
| res.statusCode = 404; | ||
| res.end('Invalid url'); | ||
| } |
There was a problem hiding this comment.
The routing logic is not structured correctly. For a request to '/', this condition url !== '/submit-expense' is true, so the server will attempt to send a 404 response. However, the block for GET / will also execute, trying to send a 200 response. This will cause a server error (ERR_STREAM_WRITE_AFTER_END). To fix this, you should use an if...else if...else structure to ensure only one response is sent per request.
src/createServer.js
Outdated
| req.on('end', () => { | ||
| const params = new URLSearchParams(body); | ||
| const newExpense = { | ||
| data: params.get('date'), |
There was a problem hiding this comment.
The key name data for the date value can be confusing. It would be clearer if the key matched the value it represents, for example, date: params.get('date').
src/createServer.js
Outdated
| return res.end('Error reading the database'); | ||
| } | ||
|
|
||
| const jsonToSave = JSON.stringify(newExpense, null, 2); |
There was a problem hiding this comment.
This implementation overwrites the db/expense.json file with every new submission, losing all previous data. To correctly save expenses, you should:
- Read the file content (
data). - Parse the JSON string from
datainto a JavaScript array. - Add the
newExpenseobject to this array. - Stringify the entire updated array and save that back to the file.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this iteration! I'm happy to approve your solution. You've successfully addressed all the critical issues from the previous review. The server's routing logic is now correct, preventing any crashes, and new expenses are properly appended to the expense.json file without overwriting existing data. The application is now fully functional and meets all the task requirements. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.