diff --git a/websockets/backend/backend.js b/websockets/backend/backend.js index de1dc3d..05782b4 100644 --- a/websockets/backend/backend.js +++ b/websockets/backend/backend.js @@ -34,6 +34,9 @@ app.listen(port, () => { app.post("/message", (req, res) => { const bodyBytes = []; + + // It's quite hard to read what this function is doing, because it does lots of things at different levels of abstraction. + // Can you think how to make this function only act at one level of abstraction (e.g. by using a middleware for some of the handling)? req.on("data", (chunk) => bodyBytes.push(...chunk)); req.on("end", () => { const bodyString = String.fromCharCode(...bodyBytes); @@ -56,6 +59,7 @@ app.post("/message", (req, res) => { } const newMessage = { messageText: body.messageText, + // Can you think of any problems with using a user-supplied timestamp here, vs calculating the timestamp on the server? timestamp: body.timestamp, likes: 0, dislikes: 0, @@ -135,6 +139,7 @@ webSocketServer.on("request", (request) => { console.log(new Date() + " Connection accepted."); activeWsConnections.push(connection); connection.on("close", function (reasonCode, description) { + // This looks like a TODO you meant to do? // put removing connection from active connections array here console.log( new Date() + " Peer " + connection.remoteAddress + " disconnected." diff --git a/websockets/frontend/client.js b/websockets/frontend/client.js index 0d4d43e..ee9ac39 100644 --- a/websockets/frontend/client.js +++ b/websockets/frontend/client.js @@ -3,6 +3,8 @@ const websocket = new WebSocket( "wss://droid-an-chat-application-backend.hosting.codeyourfuture.io" ); +// It looks like you're using websockets just to notify "there's something for you to fetch", which feels slower than including the messages in the websocket itself. +// What do you think? websocket.addEventListener("open", () => { console.log("CONNECTED"); fetchAllMessagesSince(); @@ -34,6 +36,7 @@ const postMessageToBackend = async () => { const messageText = inputMessage.value.trim(); if (!messageText) { + // This feels like it would be handy to extract a named function for "show an error message and then remove it". feedbackMessage.textContent = "Text is required."; setTimeout(() => (feedbackMessage.textContent = ""), 5000); return; @@ -140,6 +143,7 @@ const keepFetchingRatings = async () => { websocket.addEventListener("message", (mesEvent) => { const response = JSON.parse(mesEvent.data); + // How unique do you expect timestamps to be? Can you see any risks with this approach to identifying messages? if (!state.messages.some((mes) => mes.timestamp === response.timestamp)) { state.messages.push(response); render();