Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

ZA_2-Nishka_Kisten-JavaScript-Core-1-Coursework-Week3 - #65

Open
Nishka-Kisten wants to merge 2 commits into
CodeYourFuture:mainfrom
Nishka-Kisten:main
Open

ZA_2-Nishka_Kisten-JavaScript-Core-1-Coursework-Week3#65
Nishka-Kisten wants to merge 2 commits into
CodeYourFuture:mainfrom
Nishka-Kisten:main

Conversation

@Nishka-Kisten

Copy link
Copy Markdown

No description provided.

@dschouw-CYF dschouw-CYF left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well done Nishka!
Really good use of array methods and the required looping constructs!

array.push(i * 2);
i++;
}
return array.join();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nicely done!


function evenNumbersSum(n) {
// TODO
let i = 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 is really well done.
Only comment is to please try and format/indent your code consistently. 👍

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will do!

"Tottenham Court Road"
];

for (let value of tubeStations) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code works perfectly fine. Try and name your variables with meaningful names ; names like value and val could be more descriptive to convey its actual use


function getRandomNumberGreaterThan50() {
// TODO - implement using a do-while loop
let i ;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Really well done!

*/
function potentialHeadlines(allArticleTitles) {
// TODO
var result = allArticleTitles.filter((n) => n.length <= 65)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good use of filter and the conditional to check for results before returning it.
What would happen if there were no articles with a length less than 65 and someone else was using your function ? Do you think your function should always return an array, even if it is an empty array?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No.. It should probably return a message saying there are no articles that fit right?

function titleWithFewestWords(allArticleTitles) {
// TODO
}
var fewestWords = allArticleTitles[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.

Excellent algorithm with the for loop and initializing the fewestWords with the first element.
You could also use a for of loop, but your solution works!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will try it out though. Thanks!

Comment thread 2-mandatory/4-stocks.js
let sum = 0;
for(let element of closingPricesForAllStocks){
sum = element.reduce((a, b) => a + b);
tot = sum / 5;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good use of .reduce() function.
The constant literal 5 is an example of a "magic number" that should be avoided. What would happen to this calculation if the number of prices changed from 5 to any other number?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It would be wrong.. Should I use .length? I will try and do it without magic numbers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants