From 66df3f582f3b37201bbe5887f587aa8e0127af8c Mon Sep 17 00:00:00 2001 From: hhow09 Date: Mon, 27 Jan 2025 21:56:45 +0100 Subject: [PATCH 1/4] fix: async function --- backend/src/app.spec.ts | 2 +- backend/src/app.ts | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/backend/src/app.spec.ts b/backend/src/app.spec.ts index d886fe0..4acc21d 100644 --- a/backend/src/app.spec.ts +++ b/backend/src/app.spec.ts @@ -9,7 +9,7 @@ describe("App Integration Test", () => { let chatServer: ChatServer; let mongoClient: MongoClient; let clientSocket: ClientSocket; - let close: () => void; + let close: () => Promise; const mongoUri = process.env.MONGODB_URI || "mongodb://localhost:27017"; const serverPort = 3001; const serverUri = `http://localhost:${serverPort}`; diff --git a/backend/src/app.ts b/backend/src/app.ts index 0f45d37..e2a7861 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -19,17 +19,21 @@ function connectMongo(uri: string): Promise { } async function main() { - const logger = pino(); + // setup connection + // fail first if connection fail const mongoClient = await connectMongo(uri); + + // construct component + const logger = pino(); const collection = await getMongoCollection(mongoClient, dbName, collectionName); const commandService = new CommandService(logger, new ChatRepo(collection)); const chatServer = new ChatServer(logger, commandService, 3000); - const serverClose = await chatServer.listen(); + const serverClose = chatServer.listen(); // graceful shutdown - const shutdown = () => { - serverClose(); - mongoClient.close(); + const shutdown = async () => { + await serverClose(); + await mongoClient.close(); } process.on('SIGTERM', shutdown); process.on('SIGINT', shutdown); From 9b3af51e3c1923237a9e32cefbb006db46dcbb23 Mon Sep 17 00:00:00 2001 From: hhow09 Date: Mon, 27 Jan 2025 21:57:16 +0100 Subject: [PATCH 2/4] doc: add comment and README --- backend/README.md | 4 +++- backend/src/chat-server.ts | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/backend/README.md b/backend/README.md index f047084..a25e49d 100644 --- a/backend/README.md +++ b/backend/README.md @@ -48,6 +48,8 @@ src ├── chat-repo.ts └── index.ts ``` +- dependency injection is used to decouple components for (1) separation of concerns and (2) better testability. +- hierarchy: app -> chat-server -> command-service -> chat-repo -> mongodb ## MongoDB Data Modeling - Based on the requirement of `history` command (the only read operation), the data needed to be persisted is `clientId`, `operation`, and `result`. @@ -91,4 +93,4 @@ src - e.g. `5 * -3` will return error. - large number will be converted to exponential notation: e.g. `999999999999 * 999999999999` will return `9.99999999998e+23` - default threshold of exponent is `20` - - ref: https://mikemcl.github.io/decimal.js/#precision \ No newline at end of file + - ref: https://mikemcl.github.io/decimal.js/#toExpPos \ No newline at end of file diff --git a/backend/src/chat-server.ts b/backend/src/chat-server.ts index e181236..b8011b0 100644 --- a/backend/src/chat-server.ts +++ b/backend/src/chat-server.ts @@ -42,6 +42,8 @@ export class ChatServer { try { const history = await this.commandService.getHistory(socket.id); sessionLogger.info(`Returning history: ${JSON.stringify(history)}`); + // There is no need to run JSON.stringify() on objects as it (socket.io) will be done for you. + // ref: https://socket.io/docs/v4/emitting-events/#basic-emit socket.emit('history', history); } catch (error) { this.handleSocketError(socket, sessionLogger, "history", error as Error); From 11cf527e180a6b3dbc62dc52f67cc81d3673c330 Mon Sep 17 00:00:00 2001 From: hhow09 Date: Mon, 27 Jan 2025 21:58:05 +0100 Subject: [PATCH 3/4] fix: remove duplicate fn call --- backend/src/command-service.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/backend/src/command-service.ts b/backend/src/command-service.ts index 4de0cda..662fd84 100644 --- a/backend/src/command-service.ts +++ b/backend/src/command-service.ts @@ -30,9 +30,6 @@ class CommandService implements ICommandService { // evaluate evaluate a mathematical expression private evaluate(expression: string): string { - if (!isValidCommand(expression)) { - throw new Error('Invalid command'); - } return evaluate(expression); } } From 77f6aa10197dd0d806d7b89bd70f4ca0be6859fe Mon Sep 17 00:00:00 2001 From: hhow09 Date: Mon, 27 Jan 2025 22:05:16 +0100 Subject: [PATCH 4/4] refactor: restructure fn and use better naming --- backend/src/command-service.ts | 2 +- backend/src/math/evaluate.ts | 43 +++++++++++++++++++++------------- backend/src/math/types.ts | 10 ++++---- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/backend/src/command-service.ts b/backend/src/command-service.ts index 662fd84..378dc1e 100644 --- a/backend/src/command-service.ts +++ b/backend/src/command-service.ts @@ -1,7 +1,7 @@ import { Logger } from "pino"; import { CommandAndResult } from "./entities/command-result.entity"; import { IRepository } from "./repositories"; -import { isValidCommand, evaluate } from "./math/evaluate"; +import { evaluate } from "./math/evaluate"; export interface ICommandService { evaluateAndSave(clientId: string, expression: string): Promise; diff --git a/backend/src/math/evaluate.ts b/backend/src/math/evaluate.ts index 278c3de..e444974 100644 --- a/backend/src/math/evaluate.ts +++ b/backend/src/math/evaluate.ts @@ -1,33 +1,44 @@ import { Summand } from './types'; const operators = new Set(['+', '-', '*', '/']); -// isValidCommand checks if the command is a allowed mathematical expression -function isValidCommand(s: string): boolean { - s = s.replace(/ /g,''); // remove all spaces - if (s.length === 0) { +// isValidOperation checks if the operation is a allowed mathematical expression +function isValidOperation(operation: string): boolean { + const cleanedOperation = operation.replace(/ /g,''); // remove all spaces + if (cleanedOperation.length === 0) { return false; } - // allowed characters: 0-9, +, -, *, /, . - const regex = /^[\d+\-*/.]+$/; - if (!regex.test(s)) { + if (!allowedChars(cleanedOperation)) { return false; } - // operators cannot be adjacent to each other - for (let i = 0; i < s.length - 1; i++) { - if (operators.has(s[i]) && operators.has(s[i + 1])) { - return false; - } + if (!noAdjacentOperators(cleanedOperation)) { + return false; } - // operators cannot be at the end of the string - if (operators.has(s[s.length - 1])) { + if (noOperatorsAtTheEnd(cleanedOperation)) { return false; } return true; } +function allowedChars(operation: string): boolean { + return /^[\d+\-*/.]+$/.test(operation); +} + +function noAdjacentOperators(operation: string): boolean { + for (let i = 0; i < operation.length - 1; i++) { + if (operators.has(operation[i]) && operators.has(operation[i + 1])) { + return false; + } + } + return true; +} + +function noOperatorsAtTheEnd(operation: string): boolean { + return operators.has(operation[operation.length - 1]); +} + // evaluate evaluate a mathematical expression function evaluate(s: string): string { - if (!isValidCommand(s)) { + if (!isValidOperation(s)) { throw new Error('Invalid command'); } s = s.replace(/ /g,''); // remove all spaces @@ -72,4 +83,4 @@ const parseExpression = (s: string): Summand[] => { return summands; } -export { isValidCommand, evaluate }; \ No newline at end of file +export { evaluate }; \ No newline at end of file diff --git a/backend/src/math/types.ts b/backend/src/math/types.ts index cdd7580..bb8c673 100644 --- a/backend/src/math/types.ts +++ b/backend/src/math/types.ts @@ -79,7 +79,7 @@ export class Fraction { } public add(f: Fraction): Fraction { - const commonDenominator = getLcm(this.denominator, f.denominator); + const commonDenominator = getLowestCommonMultiple(this.denominator, f.denominator); const multiplyThis = commonDenominator.div(this.denominator); const multiplyF = commonDenominator.div(f.denominator); this.numerator = this.numerator.mul(multiplyThis); @@ -95,16 +95,16 @@ export class Fraction { } } -function getLcm(a: Decimal, b: Decimal): Decimal { - return a.mul(b).div(getGcd(a, b)); +function getLowestCommonMultiple(a: Decimal, b: Decimal): Decimal { + return a.mul(b).div(getGreatestCommonDivisor(a, b)); } -function getGcd(a: Decimal, b: Decimal): Decimal { +function getGreatestCommonDivisor(a: Decimal, b: Decimal): Decimal { const max = Decimal.max(a, b); const min = Decimal.min(a, b); if (max.mod(min).eq(0)) { return min; } else { - return getGcd(max.mod(min), min); + return getGreatestCommonDivisor(max.mod(min), min); } } \ No newline at end of file