Homework areas for improvement

[hynesy23]

note-taker-app/package.json

Line 9 in f796bd3

"test": "echo \"Error: no test specified\" && exit 1"

If your application has no tests you can just delete this bit

note-taker-app/src/server.js

Line 6 in f796bd3

//const { htmlRouter, apiRouter } = require("./routes/delete");

Delete any unused, commented out code

note-taker-app/src/routes/apiRoutes.js

Line 7 in f796bd3

//const { getAllNotes, saveNotes, deleteNotes, getNotesById } = require("../controllers/apiControllers");

This would have worked fine instead of line 5 - why comment it out?

note-taker-app/src/routes/apiRoutes.js

Line 9 in f796bd3

const path = require("path");

No need for path or fs here, can be removed

note-taker-app/src/routes/apiRoutes.js

Line 13 in f796bd3

//api/notes -> GET (all notes from db.json)

More commented out code that can be removed

note-taker-app/src/controllers/apiControllers.js

Line 5 in f796bd3

const database = require("../../db/db.json");

note-taker-app/src/controllers/apiControllers.js

Line 8 in f796bd3

const BASE_URL = "http://localhost:3000/api/notes";

You're not using databse of BASE_URL here, can be removed

note-taker-app/src/controllers/apiControllers.js

Line 13 in f796bd3

fs.readFileSync(path.join(__dirname, "../../db/db.json"), "utf-8")

Would be nice to see th fs methods extracted out to their own modules

note-taker-app/src/routes/htmlRoutes.js

Line 11 in f796bd3

htmlRouter.get("/", renderIndexPage);

Currently, if you hit an endpoint that is anything other than 'notes', the app breaks. You should add a star here (htmlRouter.get("/*", renderIndexPage); ) as a catch all, so if you enter an endpoint that is not /notes it will take you back to the landing/index page

[Nrasool21]

Hello Cillian, Thank you for the feedback. i have have made all the changes (Except https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/controllers/apiControllers.js#L13 Would be nice to see th fs methods extracted out to their own modules). The app is working fine in my machine. Let me know if I should make any further changes. Thank you.

…

On Wed, Jun 2, 2021 at 7:36 PM hynesy23 ***@***.***> wrote: https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/package.json#L9 If your application has no tests you can just delete this bit https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/server.js#L6 Delete any unused, commented out code https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/routes/apiRoutes.js#L7 This would have worked fine instead of line 5 - why comment it out? https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/routes/apiRoutes.js#L9 No need for path or fs here, can be removed https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/routes/apiRoutes.js#L13 More commented out code that can be removed https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/controllers/apiControllers.js#L5 https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/controllers/apiControllers.js#L8 You're not using databse of BASE_URL here, can be removed https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/controllers/apiControllers.js#L13 Would be nice to see th fs methods extracted out to their own modules https://github.com/Nrasool21/note-taker-app/blob/f796bd31a56da387f2d58a350a3c89fd744455e0/src/routes/htmlRoutes.js#L11 Currently, if you hit an endpoint that is anything other than 'notes', the app breaks. You should add a star here (htmlRouter.get("/*", renderIndexPage); ) as a catch all, so if you enter an endpoint that is not /notes it will take you back to the landing/index page — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASP7P5YBFVHOUWIAS6ADWFTTQZ2Z3ANCNFSM457IIMYQ> .

[hynesy23]

Hi Nazia,

That was quick, nice one! Don't worry too much about the fs methods, that was just me being picky ;) This looks good, much neater. Just 2 things: you don't need line 5 in apiControllers.js, and in your apiRoutes.js, the purpose of destructuring the controller functions is that so you could delete line 5 and remove 'apiFunction.' from the start of these functions names (see screenshot below:)