-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Added solution #2322
base: master
Are you sure you want to change the base?
Added solution #2322
Conversation
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.
Good start, let's improve 🐝
src/makeCalculator.js
Outdated
return this; | ||
}, | ||
|
||
add(value, result) { |
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.
Let's imagine that this is a calculator and you want to add some number, you press +2, for example, the question is where will you take the result each time to pass it as the second arg?
Methods should take only one argument and take the result that is already stored in memory
In case of add you should increase result on value like this.result += value
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.
There was no any mentions that these methods (add, subtract, divide, multiply
) can be executed directly without operate
method. Example code shows only operate
case and all the tests passed, so I didn't expect any other cases.
src/makeCalculator.js
Outdated
return result - value; | ||
}, | ||
|
||
divide(value, result) { |
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.
what happens if i pass 0 as value?)
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.
this.result
will be Infinity.
There was no any requirements about 'dividing by zero' case handling. Should I reset result to 0 or throw an error or something else?
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.
this.result
will be Infinity.There was no any requirements about 'dividing by zero' case handling. Should I reset result to 0 or throw an error or something else?
Ok, it's just for a better user experience. Yes, you can throw an error or just notify a user that they can't divide by 0
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.
👍
No description provided.