WestMidlands | SDC-NOV-2025 | Sara Tahir| Sprint 3| Middleware #65
Conversation
SaraTahir28
added
the
Needs Review
OracPrime
added
the
Review in progress
OracPrime
left a comment
OracPrime
left a comment
There was a problem hiding this comment.
Only real issue here is gitignoring package-lock.json, otherwise good
|
|
||
| // Middleware - Extracting username | ||
|
|
||
| function usernameMiddleware(req, res, next){ //middleware always has three parameters. req from client, res you send back and a next function that passes controls to next middleware |
There was a problem hiding this comment.
Sort of. If you know you are the last middleware in the chain ("endware" I guess) then you don't need the next parameter - indeed the last (res,req)=> on a route declaration is just an example of this endware pattern. And there's also a special case middleware for error handling that has four parameters.
There was a problem hiding this comment.
Thanks for the clarification.
In this case usernameMiddleware is intended to run before the route handler, so it still needs the next parameter to pass control forward.
I understand the distinction now between middleware (req, res, next) and endware (req, res). Thanks for the explanation.
.gitignore
Outdated
| @@ -0,0 +1,2 @@ | |||
| node_modules | |||
| package-lock.json No newline at end of file | |||
There was a problem hiding this comment.
You should always check in package-lock into version control. This is what allows someone else to replicate your build. node_modules is definitely an ignore though!
OracPrime
added
Reviewed
SaraTahir28
force-pushed
the
feature/middleware
branch
from
4de0e2c to
5297301
Compare
OracPrime
left a comment
OracPrime
left a comment
There was a problem hiding this comment.
Speedy changes as a result of code reviews, particularly so speedy that you clearly haven't even run it, often don't end well.
builtinmiddleware/index.js
Outdated
| // Middleware - Extracting username | ||
|
|
||
| function usernameMiddleware(req, res, next){ //middleware always has three parameters. req from client, res you send back and a next function that passes controls to next middleware | ||
| function usernameMiddleware(req, res,){ //no next as we are not passing controls to any other middleware. |
There was a problem hiding this comment.
I obviously didn't explain clearly. usernameMiddleware isn't last, because in the route there is the "final" middleware you specify inline - the (req,res)=> bit. So this one does need a next. Not least because you still call it on line 21. If this was in a professional team environment you would be getting an email from me saying "that code you've just pushed to github: have you even run it once?" because line 21 is clearly going to error.
There was a problem hiding this comment.
I misunderstood your earlier point about middleware vs endware and removed next incorrectly. I will be careful next time.
OracPrime
added
Reviewed
SaraTahir28
added
the
Needs Review
OracPrime
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
This PR includes two working versions of the middleware application as required by the exercise:
1.Original version (manual JSON parsing)
2.Updated version (Express’s built‑in JSON parser)
Questions
I have no Questions. Thankyou.