Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions src/makeCalculator.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,43 @@
* @return {object}
*/
function makeCalculator() {
// write code here
}
let result = 0;

const add = (a, b) => a + b;
const subtract = (a, b) => a - b;
const multiply = (a, b) => a * b;
const divide = (a, b) => a / b;

const operate = function (callback, num) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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').

if (callback === add) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

result += num;
} else if (callback === subtract) {
result -= num;
} else if (callback === multiply) {
result *= num;
} else if (callback === divide) {
result /= num;
}

return this;
};
Comment on lines +14 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 () {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

result = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also violates checklist item #2: result is reset using result = 0; instead of a compound assignment form, despite updating the existing result value.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 =.


return this;
};
Comment on lines +28 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #3 again: reset is defined as a function expression instead of using method shorthand syntax inside the returned object.


return {
get result() {
return result;
},
add,
subtract,
multiply,
divide,
operate,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

reset,
};
}
module.exports = makeCalculator;
Loading