middleWord complete #16
Conversation
|
this was a test for first function ! |
| } */ | ||
| //best Lor solution i was waiting to see it! | ||
| var resultArray = capitals.map(arrayElement => `${arrayElement.city} is the capital of ${arrayElement.country}`); | ||
| return (resultArray); | ||
| } |
There was a problem hiding this comment.
In this solution, you're not making use of the formatter() callback provided in the starter code. Happy to discuss how you make use of this on Saturday.
| } | ||
|
|
||
| function even( numbers ){ | ||
| // numbers is an array of numbers | ||
| // return a new array that contains only even numbers from the input array | ||
| // hint: you may want to use the modulus operator '%' | ||
| //numbers.filter(i => (i%2 ==0)?i:0 ); | ||
| //for(var i=0;i<numbers.length;i++){ | ||
| return numbers.filter(i => !(i&1)) | ||
| } |
There was a problem hiding this comment.
Whilst the solution works fine and passes the test (as do all the others), this one could be written differently to make it more readable/understandable at a later date. Think of code as something humans need to be able to read, rather than the machine, as it makes supporting solutions much easier in the future.
| if (copy[i].make === 'Ford') | ||
| copy[i].colour = colour; | ||
| } | ||
| return copy; | ||
| } |
There was a problem hiding this comment.
This is an interesting approach to the problem. Definitely one where you could consider an alternative approach e.g. create copy array as first step, then loop through copy to amend its contents.
| } | ||
| } | ||
| return newObj; | ||
| }//lloc |
There was a problem hiding this comment.
Whilst this one works, give some thought to what would happen if the test array contained BMW, Mercedes and Fiat vehicles. Hard coding values into a function is rarely a good idea, as it limits their reuse, which is a functions' main purpose. I'd be happy to take you through a different (reusable) solution tomorrow.
| @@ -128,7 +221,13 @@ function factorial( int ) { | |||
|
|
|||
| // calculate and return the factorial of int | |||
| // note: factorial of 0 is 1 | |||
| } | |||
| var factorial = int =>(int===0) ? 1 : int*factorial(int-1); | |||
There was a problem hiding this comment.
With this solution, whilst it worked, you've broken out of the function structure, and created a variable on the global scope. For the purposes of this exercise, it's not the best answer, thought you should be able to wrap it in a function to bring it in line with the way you've completed the other functions.
| @@ -144,5 +243,6 @@ module.exports = { | |||
| average, | |||
| paintShop, | |||
| sales, | |||
| secondLargest | |||
| secondLargest, | |||
| factorial | |||
| }; | |||
There was a problem hiding this comment.
General comments:
- A discussion on code style would be a good idea for all students, as things like white space, nesting and variable naming would benefit everyone.
- It's a good idea to remove any unused or commented out sections before submitting a PR, as it aids readability for a reviewer.
| var secMax = arrCopy.sort((a, b)=>b - a)[1] | ||
| return arr.indexOf(secMax); | ||
|
|
||
| } |
There was a problem hiding this comment.
Very clever solution and way simpler than what I came up with. Mahsa 1 - 0 Stack Overflow...
No description provided.