Glasgow | 26 March SDC | Mohammed Abdoon| Sprint 3 | Middleware exercises#105
Glasgow | 26 March SDC | Mohammed Abdoon| Sprint 3 | Middleware exercises#105M-Abdoon wants to merge 2 commits into
Conversation
M-Abdoon
added
the
Needs Review
| const status = this.user ? `Logged in: ${this.user}` : "Anonymous"; | ||
| const count = this.entries.length; | ||
| const itemType = count === 1 ? "item" : "items"; | ||
| const details = count > 0 ? `[${this.entries.map(e => `"${e}"`).join(", ")}]` : "[]"; |
There was a problem hiding this comment.
What if an element of entries contains a double quote character?
Have you considered using a built-in JS function to simplify the encoding task?
| extractCredentials() { | ||
| this.user = this.req.get("X-Username"); | ||
| return this; | ||
| } | ||
|
|
||
| processPayload() { |
There was a problem hiding this comment.
These middleware functions/methods do not quite adhere to the Express middleware pattern.
| const raw = this.req.body; | ||
| if (!raw || !raw.trim()) { | ||
| return this; | ||
| } |
There was a problem hiding this comment.
According to the spec, the middleware is expected to "parse the request POST body" (instead of consuming the parsed req.body content).
Suggestion: Look up (ask AI)
How to implement an Express middleware to parse POST body without using Express body parsers?
| const app = express(); | ||
| const port = 3001; | ||
|
|
||
| app.use(express.text({ type: "application/x-www-form-urlencoded" })); |
There was a problem hiding this comment.
One of your middleware is expected to parse the raw POST body. You should not need this.
| buildReply() { | ||
| const status = this.user ? `Logged in: ${this.user}` : "Anonymous"; | ||
| const count = this.entries.length; | ||
| const itemType = count === 1 ? "item" : "items"; | ||
| const details = count > 0 ? `[${this.entries.map(e => `"${e}"`).join(", ")}]` : "[]"; | ||
|
|
||
| return `Status: ${status}\nQuery: ${count} ${itemType}\nData: ${details}`; | ||
| } | ||
|
|
||
| send() { | ||
| this.res.send(this.buildReply()); | ||
| } |
There was a problem hiding this comment.
These two operations should not be tightly coupled with the middleware you implemented. Supposedly in the off-the-shelf-middleware, you could just replace the middleware functions and everything else works the same.
There was a problem hiding this comment.
According to the spec, this app is expected to behave like the app implemented in "custom-middleware".
cjyuan
added
Reviewed
2 participants
Learners, PR Template
Self checklist