-
Notifications
You must be signed in to change notification settings - Fork 15
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
work on web-calculator #15
base: master
Are you sure you want to change the base?
Conversation
@@ -46,8 +64,11 @@ | |||
<input type="number" class="form-control" name="numberTwo"> | |||
</div> | |||
<div> |
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 part looks solid.
@@ -13,16 +13,34 @@ | |||
|
|||
<script> | |||
var calculator = { | |||
addNumbers: function () { | |||
addNumbers: function (numberOne, numberTwo) { |
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.
Having the parameters defined on line 16 is not needed. We do not take input into the function. Instead we get the values to work with by taking them from values in the form fields. Lines 18 and 19 for example are getting the values we will use.
}, | ||
subtractNumbers: function () { | ||
subtractNumbers: function (numberOne, numberTwo) |
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.
The formatting makes this code look like it needs to be reformatted.
Tabbing, spacing, new lines, indentation, etc are all key to readable code.
You can use the built-in formatting in VS Code.
To use the formatter, go to the View > Command Palette:
Then choose the "Format Document" option.
This should make the code have an easier to read formatting. Push that code up as a new commit.
var numberTwo = document.forms.inputs.numberTwo.value; | ||
document.forms.inputs.result.value = parseInt(numberOne) / parseInt(numberTwo); | ||
}, | ||
|
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.
The code above all looks correct. The formatting is not required for code to work. It is important to learn for readability by you and others.
No description provided.