-
Notifications
You must be signed in to change notification settings - Fork 0
Refact code test #4
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
Conversation
WalkthroughAs mudanças atualizam o Changes
Sequence Diagram(s)sequenceDiagram
participant C as Cliente
participant P as ExpressionParser
participant S as ExpressionSolver
C->>P: parse(expressão)
P-->>C: tokens
C->>S: solve(tokens)
S-->>C: resultado
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
main.ts (1)
192-193: Remove or finalize the commented-out debug statement.For clarity, remove the commented-out
console.logif it’s no longer necessary.-// console.log(solver.solve("1 + 2 * 3"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.MD(1 hunks)main.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
main.ts
[error] 143-143: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 155-155: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (3)
README.MD (1)
2-4: No functional changes introduced; additions accepted.This addition of blank lines and a testing message in the README doesn't introduce any functional concerns. Keep it up!
main.ts (2)
1-4: Good use of a structured operation definition.Defining the operation signature with a priority level and a solve method is straightforward and maintainable.
6-9: Interface defines a clear contract.The interface neatly encapsulates the parser's responsibility. This separation of concerns is beneficial for maintainability.
| } | ||
| #addDecimalPoint() { | ||
| if (this.#hasDecimal || !this.#isValidNumber()) | ||
| throw "Número decimal mal formado."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Prefer throwing Error objects instead of strings.
Using throw new Error(...) ensures stack traces are correctly captured and aligns with standard JavaScript/TypeScript practices.
-throw "Número decimal mal formado.";
+throw new Error("Número decimal mal formado.");
-throw "Parêntese fechado sem abertura correspondente.";
+throw new Error("Parêntese fechado sem abertura correspondente.");
-throw `Operador '${operator}' mal posicionado na expressão.`;
+throw new Error(`Operador '${operator}' mal posicionado na expressão.`);
-throw `Caractere inválido na expressão: '${token}'`;
+throw new Error(`Caractere inválido na expressão: '${token}'`);
-throw `Operador '${this.#lastToken}' mal posicionado na expressão.`;
+throw new Error(`Operador '${this.#lastToken}' mal posicionado na expressão.`);
-throw "Parêntese aberto sem fechamento correspondente.";
+throw new Error("Parêntese aberto sem fechamento correspondente.");Also applies to: 63-63, 77-77, 102-102, 110-110, 116-116
| const firstNumber = Number(this.#output.pop()); | ||
| const operator = this.#operators.pop(); | ||
|
|
||
| if (operator == null || !this.#operations.hasOwnProperty(operator)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Switch from using hasOwnProperty to Object.hasOwn().
Avoid calling hasOwnProperty on an instance; using Object.hasOwn() is safer and recommended. Also convert string throws to Error objects where applicable.
-if (operator == null || !this.#operations.hasOwnProperty(operator))
- throw "Operation Not Supported.";
+if (operator == null || !Object.hasOwn(this.#operations, operator))
+ throw new Error("Operation Not Supported.");
...
-return this.#operations.hasOwnProperty(token);
+return Object.hasOwn(this.#operations, token);Also applies to: 155-155
🧰 Tools
🪛 Biome (1.9.4)
[error] 143-143: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
Summary by CodeRabbit
Documentação
Novos Recursos