London-10-Ahmed -Adam | JavaScript-Core-1-Coursework-Week3 - #226
London-10-Ahmed -Adam | JavaScript-Core-1-Coursework-Week3#226ahmedsaif2002 wants to merge 5 commits into
Conversation
| */ | ||
|
|
||
| function getTemperatureReport(cities) { | ||
| function getTemperatureReport(cities) { |
There was a problem hiding this comment.
This looks very good 👍
Try to keep an eye on indentation and spacing. It sounds like a minor point - can make it much easier for other developers to read your code 😄
| @@ -5,7 +5,13 @@ | |||
| Implement the function below, which will return a new array containing only article titles which will fit. | |||
| */ | |||
| function potentialHeadlines(allArticleTitles) { | |||
There was a problem hiding this comment.
Again, the implementation looks good. Just think about indentation 😄
| if(allArticleTitles[i].split(" ").length< FewestWords.split(" ").length){ | ||
| FewestWords = allArticleTitles[i]; | ||
| } | ||
| console.log(allArticleTitles); |
There was a problem hiding this comment.
Do you need this console.log?
| @@ -14,7 +20,13 @@ function potentialHeadlines(allArticleTitles) { | |||
| (you can assume words will always be seperated by a space) | |||
| */ | |||
| function titleWithFewestWords(allArticleTitles) { | |||
There was a problem hiding this comment.
I like this implementation! Looks very good.
Just keep an eye on how you're naming variables. Developers use conventions for naming variables, functions, etc. that make it easier for other developers to read your code. I found some conventions listed on this page that you could take a look at: https://www.30secondsofcode.org/articles/s/javascript-naming-conventions
| for(let article of allArticleTitles){ | ||
| for (let char of article) | ||
| { | ||
| if(parseInt(char)){ |
There was a problem hiding this comment.
This is a good attempt 😄
There may be better ways to check if a character is a number though. I think your code will not work if the article title contains a 0. For example, take a look at this code:
let num = '0';
if(parseInt(num)) {
console.log('num is a number');
} else {
console.log('num is not a number');
}| } | ||
|
|
||
| } | ||
| if(count==0){ |
There was a problem hiding this comment.
This is a good check 👍
| let sum =0; | ||
| let count =0; | ||
| for(let i = 0; i< allArticleTitles.length; i++){ | ||
| if(typeof allArticleTitles[i]==="string"){ |
There was a problem hiding this comment.
Is this check needed?
| @@ -35,6 +35,16 @@ const CLOSING_PRICES_LAST_5_DAYS_FOR_ALL_STOCKS = [ | |||
| */ | |||
| function getAveragePrices(closingPricesForAllStocks) { | |||
There was a problem hiding this comment.
Great job on this one! 👍
Again, just be careful with your indentation 😄
| function getPriceChanges(closingPricesForAllStocks) { | ||
| const priceChanges = []; | ||
|
|
||
| for (let i = 0; i < closingPricesForAllStocks.length; i++) { |
There was a problem hiding this comment.
Nice work!
I like the way you're using extra variables to make the code very clear and easy to read.
| @@ -64,11 +86,31 @@ function getPriceChanges(closingPricesForAllStocks) { | |||
| The price should be shown with exactly 2 decimal places. | |||
| */ | |||
| function highestPriceDescriptions(closingPricesForAllStocks, stocks) { | |||
There was a problem hiding this comment.
This looks good to me.
Can you think about ways to make this a little easier to read?
One suggestion might be to have a separate function to calculate the highest price in an array of prices. Then we can call that new function from this one.
| // return radioFrequencie; | ||
| // } | ||
|
|
||
| function getAllFrequencies() { |
| */ | ||
| // `getStations` goes here | ||
|
|
||
| function getStations() { |
There was a problem hiding this comment.
This looks great.
As an extra exercise - can you re-write this using the filter array method?
|
Great job on this coursework! |
No description provided.