Glasgow | 25-SDC-Nov | Nataliia Volkova | Sprint 3 | Middleware #61
Conversation
Nataliia74
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.
Some comments to take note of, but I don't think any changes are required
| function authenticationCheck(req, res, next) { | ||
| const username = req.header("X-Username"); | ||
|
|
||
| req.username = username ? username : null; |
There was a problem hiding this comment.
This code works, but it would probably be more normal to see it written as either
req.username = username ?? null;
or
req.username = username || null;
A useful exercise would be to understand the difference between these two forms (and why the first one is better)
| app.use(express.json()); | ||
| app.use(cors()); | ||
|
|
||
| function authenticationCheck(req, res, next) { |
There was a problem hiding this comment.
Naming is hard. I would suggest that this isn't an authenticationCheck because it doesn't do anything different if the header isn't there. With a name with check in it I'd expect a 401 response if there isn't the required header
| }, | ||
| "keywords": [], | ||
| "author": "", | ||
| "license": "ISC", |
There was a problem hiding this comment.
Nice! Always good to include a licence in anything you're committing to github so others know where they stand (although I suspect npm init did this for you?)
| next(); | ||
| } | ||
|
|
||
| // function bodyMessageCheck(req, res, next) { |
There was a problem hiding this comment.
Not a huge fan of commented-out code. Never sure why it's here [in this case it's obvious, but in general commented-out code without explanation just confuses people]
OracPrime
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
Middleware exercises 4