Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NW6 | Rabia Avci | Module Servers | [TECH ED] Chat Server API Project | Week 2 #166

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

RbAvci
Copy link

@RbAvci RbAvci commented Apr 18, 2024

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Some notes for my PR:

  • An Express.js server for a basic chat system:

  • Used Middlewares: like express.urlencoded, express.json, and cors to handle different types of requests and enable Cross-Origin Resource Sharing (CORS).

  • For Data: an array called messages to act as the data store for chat messages.

  • Routes:

Root Route: Serves index.html at the root URL ("/").
GET /messages: Retrieves all messages.
GET /messages/search: Filters messages by text query parameter.
GET /messages/latest: Retrieves the latest 10 messages.
GET /messages/:id: Retrieves a specific message by ID.
POST /messages: Adds a new message.
PUT /messages/:id: Updates a message by ID.
DELETE /messages/:id: Deletes a message by ID.

Copy link
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, good job!


// GET a specific message by id
app.get("/messages/:id", (req, res) => {
const message = messages.find((p) => p.id === parseInt(req.params.id));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many times will this call the parseInt function? If you call parseInt on the same value multiple times, will it return different values? Can you think of a way to call it less often?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To call parseInt less often we can parse the ID outside of the find function and store it in a variable 👍 Thanks!


app.post("/messages", (req, res) => {
const newId = (lastId += 1);
if (!req.body.from) return res.status(422).json({ message: "From field is required" }); //A 422 status code indicates that the server was unable to process the request because it contains invalid data.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though your code here is correct and works, a lot of programmers will always add {}s around if bodies, even if they're just one statement - have a read of https://www.synopsys.com/blogs/software-security/understanding-apple-goto-fail-vulnerability-2.html for some explanation as to why :)

Copy link

sonarcloud bot commented Apr 19, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants