London | July SDC | Ali Qassab | Sprint 3 | Middleware Exercises #48
Conversation
AliQassab
added
📅 Sprint 3
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Looking good, though I have a suggestion of something to think about to help practise writing even more reusable middleware
I've also left a general comment about remembering how to make good PRs
sprint3_exercises/websocket.js
Outdated
There was a problem hiding this comment.
Remember when committing files to make a PR to only include the relevant ones. As this is about the middleware task, do you need to be committing this one?
There was a problem hiding this comment.
No, I don’t need it here. I deleted it.
| app.post("/", (req, res) => { | ||
| // Validate that body is an array | ||
| if (!Array.isArray(req.body)) { | ||
| return res.status(400).send("Request body must be a JSON array"); |
There was a problem hiding this comment.
Because you're using the express.json middleware, does that already check something is a JSON array body? Are you giving the correct response here?
There was a problem hiding this comment.
express.json only parses and validates JSON syntax; it does not enforce that the body is an array. We still need separate validation for shape and element types.
Yes, the 400 responses for “must be a JSON array” and “all elements must be strings” are correct for validation failures after parsing
sprint3_exercises/middleware.js
Outdated
| } | ||
|
|
||
| // Check if all elements are strings | ||
| const allStrings = parsed.every((item) => typeof item === "string"); |
There was a problem hiding this comment.
What is the goal of this middleware - it seems to do two things.
Here you're combining JSON parsing and checking the input is strings. As middleware aims to allow re-use of code, what change could you make to this that would allow you to re-use these separate checks more easily?
There was a problem hiding this comment.
Goal of the middleware: Parse JSON POST bodies and validate an array of strings.
The change I made:
- Split it into two pieces:
- express.json() for parsing JSON.
- validateArrayOfStrings for validation only.
LonMcGregor
added
Reviewed
LonMcGregor
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
Complete Sprint 3 prep exercises - WebSockets and Middleware
Questions
Ready to review!