Birmingham | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Chat App by JanefrancessC · Pull Request #84 · CodeYourFuture/Module-Decomposition

JanefrancessC · 2026-05-26T10:16:12Z

Learners, PR Template

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist

Backend link
Frontend link

Additional Feature

I added the like and dislike buttons, which are incremented on click.

cjyuan

Code works well on normal circumstances. Well done.
Code could use some documentation (comments).

A useful rule of thumb is to ask yourself whether you could still explain or figure out how the code works few months from now. If the answer is uncertain, consider adding a comment to help future readers understand the intent and reasoning behind it.
The README has this requirement:

It must also support at least one additional feature.

In the PR description, can you list the extra features you implemented?

cjyuan · 2026-05-28T18:17:08Z

+const port = process.env.PORT || 3000;
+let nextMessageId = 1;
+const messages = [


Better to declare global constants and variables before const app = express() because lines 5-6 are already logic to setup the server (app).

cjyuan · 2026-05-28T18:23:55Z

+});
+
+app.post("/", (req, res) => {
+  const { message, user } = req.body;


It's good practice to validate all request inputs.

Note: A more robust implementation

Should also handle the case where req.body is null, which would cause const { text, sender } = req.body; to throw.

In this exercise, let's assume the request is always coming from the code in script.js. So no change needed.

cjyuan · 2026-05-28T18:26:40Z

+    !message.trim() ||
+    !user.trim()
+  ) {
+    res.status(400).json({ error: `Message and user are required!` });
+    return;
+  }
+
+  const newMessage = {
+    id: nextMessageId++,
+    message,
+    user,


It's a bit inconsistent that message containing only space characters are not allowed but leading and trailing space characters are acceptable.

cjyuan · 2026-05-28T18:28:49Z

+  messages.push(newMessage);
+  while (callbacksForNewMessages.length > 0) {
+    const callback = callbacksForNewMessages.pop();
+    callback([messages[messages.length - 1]]);


Wouldn't sending newMessage shorter in syntax?

cjyuan · 2026-05-29T00:13:54Z

+    state.messages = state.messages.map((msg) =>
+      msg.id === updatedMessage.id ? updatedMessage : msg,
+    );


Note: Not very efficient approach to update one number. (Change is optional, as this is an extra feature).

I didn't give it much thought since it's classwork, but I'll rectify this so I don't have to loop through all the messages to update one message.

cjyuan · 2026-05-29T00:18:22Z

+async function dislikeMessage(msgId) {
+  try {
+    const response = await fetch(`${serverURL}/${msgId}/dislike`, {
+      method: "POST",
+    });
+
+    if (!response.ok) {
+      feedbackEl.innerHTML = `<p>${await response.text()}</p>`;
+      return;
+    }
+    const updatedMessage = await response.json();
+
+    state.messages = state.messages.map((msg) =>
+      msg.id === updatedMessage.id ? updatedMessage : msg,
+    );
+    displayMessages();
+  } catch (error) {
+    console.error(error.message);
+  }
+}


The code in dislikeMessage() and likeMessage() is nearly identical and could be refactored to reduce duplication.

The same also applies to their counterparts on the server side.

I will also modify this by creating a generic function, then using a conditional statement to increment either the like or dislike button.

cjyuan · 2026-05-29T00:22:50Z

+      (msg) =>
+        `<div>
+            <p>
+                <strong>${msg.user}: ${msg.message} </strong>


What if the user's name or message contains characters like < or >? For example, <javascript src="..."></script>.

This is quite interesting about innerHTML. Thank you for pointing this out; I never thought about users passing HTML tags. I will rectify this by creating DOM elements and assigning them values using textContent

cjyuan · 2026-05-29T00:32:44Z

+  }
+}
+
+function displayMessages() {


If this function is design to accept messages as a parameter, it would make the rendering logic easier to develop and test independently.

As a general rule, prefer passing data through parameters instead of relying on global variables.

I will modify this to accept messages as a parameter, rather than a global variable.

cjyuan · 2026-05-29T00:48:19Z

+    feedbackEl.innerHTML = `<p>Message sent!</p>`;
+
+    setTimeout(() => {
+      feedbackEl.innerHTML = "";
+    }, 3000);


Could consider isolating the logic for showing feedback in a function showFeedback(msg, timeoutDuration).

cjyuan · 2026-05-29T00:58:38Z

+  if (longPoll) {
+    keepFetchingMessages();
+  } else {
+    setTimeout(keepFetchingMessages, 2000);


2000 is a magic number and best practice is to represent all magic numbers as named constants.

JanefrancessC · 2026-05-30T09:18:55Z

Code works well on normal circumstances. Well done.

Code could use some documentation (comments).
A useful rule of thumb is to ask yourself whether you could still explain or figure out how the code works few months from now. If the answer is uncertain, consider adding a comment to help future readers understand the intent and reasoning behind it.

The README has this requirement:

It must also support at least one additional feature.

In the PR description, can you list the extra features you implemented?

Thank you for the thorough feedback. I have edited the PR to include the extra feature and will modify my code per the review.

JanefrancessC added 2 commits May 21, 2026 19:35

feat: Implement basic chat-app concept

90e2c3b

feat: Implement like and dislike buttons logic

1518e8b

JanefrancessC added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Decomposition The name of the module. labels May 26, 2026

JanefrancessC added 4 commits May 26, 2026 11:26

chore: cleanup code

c310b9c

fix: Remove dotenv since not used

9b80f19

fix: Modify endpoints to accomodate like feature

85ad07e

fix: edit URL

b393959

cjyuan reviewed May 29, 2026

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels May 29, 2026

Uh oh!

Conversation

JanefrancessC commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Learners, PR Template

Changelist

Additional Feature

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JanefrancessC commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JanefrancessC commented May 26, 2026 •

edited

Loading