Solution - #3070
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still checklist violations that are considered required, not optional. In src/makeCalculator.js, you reassign result using a full expression like result = result + ... instead of a compound operator result += ..., which conflicts with [CHECKLIST ITEM #2]; update those lines so that any update to result uses +=, -=, *=, or /= as appropriate. Also in src/makeCalculator.js, both operate and reset are defined as standalone function expressions rather than using method shorthand on the returned object, which violates [CHECKLIST ITEM #3]; refactor them so the returned object uses operate(arg) { ... } and reset() { ... } style methods. These are critical because the checklist is part of the core requirements, even though your overall functionality (closure-based result, chainable operate/reset, and use of callbacks like add/subtract/multiply/divide) is otherwise correct.
As a robustness improvement (not strictly a blocker), consider adding a typeof callback === 'function' guard inside operate so passing a non-function doesn’t throw an error; for example, early-return this if the callback isn’t a function. Once you address the compound assignment and method shorthand issues, your solution will align with both the functional and style requirements, and the overall design you already have will remain strong.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| const divide = (a, b) => a / b; | ||
|
|
||
| const operate = function (callback, num) { | ||
| result = callback(result, num); |
There was a problem hiding this comment.
This violates checklist item #2: Use arithmetic operators with assignment operator '=' if you want to add something to existing value. Here result is reassigned via a full expression; consider using compound assignment like result += ... when updating from its current value.
| const multiply = (a, b) => a * b; | ||
| const divide = (a, b) => a / b; | ||
|
|
||
| const operate = function (callback, num) { |
There was a problem hiding this comment.
This violates checklist item #3: if you are creating a method in the object, you don't need to use function keyword, use shortcut instead. Defining operate as a separate const function rather than a method on the returned object uses the bad pattern from the checklist.
| return this; | ||
| }; | ||
|
|
||
| const reset = function () { |
There was a problem hiding this comment.
This violates checklist item #3: if you are creating a method in the object, you don't need to use function keyword, use shortcut instead. reset is also defined as a standalone function instead of using method shorthand on the returned object.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still explicit checklist violations that are considered critical for this task. Your calculator’s functionality is essentially correct, but result is being updated with full reassignment (e.g., result = callback(result, num) and result = 0) instead of using compound assignment operators as required by [CHECKLIST ITEM #2]; this happens where you update result after applying the callback and where you reset it. To fix this, redesign the way operate applies callbacks so that result can be updated via +=, -=, *=, or /= in a way that matches the intended arithmetic behavior (for example, making the callbacks return the delta or factor you apply with a compound operator). In addition, operate and reset are currently declared as standalone function expressions and then attached to the returned object, which conflicts with [CHECKLIST ITEM #3] that requires using object method shorthand; instead, define them directly inside the returned object like operate(callback, num) { ... } and reset() { ... }. These style requirements are part of the core criteria for this assignment, so they must be satisfied before your solution can pass. Once you address the compound assignment for result and convert operate/reset to shorthand methods, your implementation should fully meet both the functional and style requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| const divide = (a, b) => a / b; | ||
|
|
||
| const operate = function (callback, num) { | ||
| result = callback(result, num); |
There was a problem hiding this comment.
This violates checklist item #2: "[CODE STYLE] - use arithmetic operators with assignment operator = if you want to add something to existing value". Here result is reassigned using result = callback(result, num) instead of a compound operator like +=, -=, *=, or /= when updating the existing result value.
| const operate = function (callback, num) { | ||
| result = callback(result, num); | ||
|
|
||
| return this; | ||
| }; |
There was a problem hiding this comment.
This violates checklist item #3: "[CODE KNOWLEDGE] - if you creating a method in the object, you don't need to use function keyword, use shortcut instead." operate is declared as a standalone function expression rather than as a shorthand method on the returned object.
| }; | ||
|
|
||
| const reset = function () { | ||
| result = 0; |
There was a problem hiding this comment.
This also violates checklist item #2: result is reset using result = 0; instead of a compound assignment form, despite updating the existing result value.
| const reset = function () { | ||
| result = 0; | ||
|
|
||
| return this; | ||
| }; |
There was a problem hiding this comment.
This violates checklist item #3 again: reset is defined as a function expression instead of using method shorthand syntax inside the returned object.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution under an educational override so you can keep progressing while you refine these patterns. You’ve done a solid job capturing the calculator’s core behavior: internal result state, operate and reset that are chainable, and arithmetic methods that work correctly with the stored result. The main issues to improve are that operate still special-cases add/subtract/multiply/divide instead of working with any function (you should only need to check typeof callback === 'function' and then apply it generically to result and num), and that both operate and reset are defined as separate const functions instead of using object method shorthand in the returned object [CHECKLIST ITEM #3]. Also, reset currently uses result = 0;, which conflicts with the requirement to update result using compound assignments [CHECKLIST ITEM #2]; a good learning exercise is to redesign how reset reaches 0 while still using +=, -=, *=, or /=.
Overall, your structure is on the right track; focusing on generic callback handling in operate and method/style consistency will bring it fully in line with the requirements. Keep going—these are subtle, stylistic constraints that are great practice for writing clean, flexible JavaScript APIs.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const multiply = (a, b) => a * b; | ||
| const divide = (a, b) => a / b; | ||
|
|
||
| const operate = function (callback, num) { |
There was a problem hiding this comment.
This operate implementation violates the override instruction: "The operate method should not check that the callback provided is one of the calculator's own methods (add, subtract, multiply, divide). Any function can be passed...". Currently you compare callback to add/subtract/multiply/divide, so operate will ignore any other function. Redesign operate to work with any function without special-casing these methods (you may only check typeof callback === 'function').
| const divide = (a, b) => a / b; | ||
|
|
||
| const operate = function (callback, num) { | ||
| if (callback === add) { |
There was a problem hiding this comment.
The chain of if (callback === add) ... else if ... makes operate depend on specific calculator methods, which conflicts with the requirement that any function can be passed. It should instead apply the callback generically to result and num without checking which function it is.
| return this; | ||
| }; | ||
|
|
||
| const reset = function () { |
There was a problem hiding this comment.
reset is declared as const reset = function () { ... }, then attached to the returned object, which matches the BAD EXAMPLE in checklist item #3. Define it as a method in the returned object using shorthand (reset() { ... }) instead of a separate function expression.
| }; | ||
|
|
||
| const reset = function () { | ||
| result = 0; |
There was a problem hiding this comment.
result = 0; uses full reassignment, which conflicts with checklist item #2 and the previous review requirement to use compound assignment operators (+=, -=, *=, /=) when updating result. Adjust your design so resetting result uses a compound operator rather than direct =.
| subtract, | ||
| multiply, | ||
| divide, | ||
| operate, |
There was a problem hiding this comment.
In the returned object, operate is currently just a reference to a standalone function declared above. Checklist item #3 and the previous review require defining methods using object method shorthand (e.g., operate(callback, num) { ... }) directly inside this object, rather than as separate const function expressions.
No description provided.