New solution to match updated test syntax - #364
Conversation
See separate PR "Removed array syntax in multiply test TheOdinProject#359" TheOdinProject#359
codyloyd
left a comment
codyloyd
left a comment
There was a problem hiding this comment.
this looks fine... but i have a couple of very nitpicky requests.
| }; | ||
| const multiply = function(...args){ | ||
| let sum = 1 | ||
| for (let i = 0; i < args.length; i++) { |
There was a problem hiding this comment.
this is really nitpicky.. but can you fix the indentation on this line please.
There was a problem hiding this comment.
I've made both changes through the 'edit' function here now. Hope that works, otherwise let me know if I need to open a new PR.
| : 0; | ||
| }; | ||
| const multiply = function(...args){ | ||
| let sum = 1 |
There was a problem hiding this comment.
very nitpicky: can we call this variable product
"product" is the result of multiplying numbers.. "sum" is for adding. this doesn't matter toooo much, but since this is example code we're going to have thousands of students looking at... we want it to be as clean and correct as possible.
There was a problem hiding this comment.
Good point; no problem, will do. Thanks!
|
we're good to go! thanks |
New solution to match updated test syntax
New solution to match updated test syntax
New solution to match updated test syntax
New solution to match updated test syntax
New solution to match updated test syntax
2 participants
See separate PR "Removed array syntax in multiply test #359"
#359
Because
Updated the solution for the
multiplyfunction with one that does not take arrays as input.This PR
There may be more elegant ways to write this function, but it passes the test as long as the [brackets] are removed.
Issue
Closes #XXXXX
Additional Information
Pull Request Requirements
location of change: brief description of changeformat, e.g.01_helloWorld: Update test casesBecausesection summarizes the reason for this PRThis PRsection has a bullet point list describing the changes in this PRIssuesectionsolutionsbranch, I have created another PR (and linked it to this PR).