London | 25-SDC-July | Fatma Arslantas | Sprint 3 | Middleware exercises #30
Conversation
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
AFatmaa
added
the
Needs Review
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
1 similar comment
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
AFatmaa
changed the title
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
4 similar comments
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
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 improve the reusability of the middleware you have written
| return res.status(400).send("Error: Body must be a JSON array."); | ||
| } | ||
|
|
||
| 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.
Hi @LonMcGregor,
You're right! This middleware was handling two responsibilities: parsing JSON and validating that the input is a string array.
Middleware should ideally do one specific job, so I refactored it into two separate middlewares: one for JSON parsing and another for string validation.
This makes the code cleaner and easier to reuse in other routes.
Thank you for your feedback 🌸
LonMcGregor
added
Reviewed
|
Hi @LonMcGregor, Thank you for the suggestion! 🙌 |
AFatmaa
added
Needs Review
LonMcGregor
added
Complete
|
Thank you @LonMcGregor 🤗 |
2 participants
Learners, PR Template
Self checklist
Changelist
This PR adds two small Express applications for the Decomposition prep task.
The first app includes two custom middlewares:
The second app replaces one custom middleware with the built-in express.json() middleware.
It parses JSON automatically.
Questions
I don’t have any questions. Thank you.