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

London10_Anna-Hrychaniuk_JavaScript-Core-1-Coursework-Week3 - #232

Open
AnnaFYZ wants to merge 6 commits into
CodeYourFuture:mainfrom
AnnaFYZ:main
Open

London10_Anna-Hrychaniuk_JavaScript-Core-1-Coursework-Week3#232
AnnaFYZ wants to merge 6 commits into
CodeYourFuture:mainfrom
AnnaFYZ:main

Conversation

@AnnaFYZ

@AnnaFYZ AnnaFYZ commented Mar 5, 2023

Copy link
Copy Markdown

No description provided.

@@ -12,7 +12,13 @@
*/

function getTemperatureReport(cities) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good job 👍

@@ -5,7 +5,13 @@
Implement the function below, which will return a new array containing only article titles which will fit.
*/
function potentialHeadlines(allArticleTitles) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice one 😄
As an extra exercise, could you try re-writing this with the filter array method?

@@ -23,15 +39,30 @@ function titleWithFewestWords(allArticleTitles) {
(Hint: remember that you can also loop through the characters of a string if you need to)
*/
function headlinesWithNumbers(allArticleTitles) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very nice and simple solution.

Comment thread 2-mandatory/3-stocks.js
@@ -34,7 +34,18 @@ const CLOSING_PRICES_LAST_5_DAYS_FOR_ALL_STOCKS = [
Functions can help with this!
*/
function getAveragePrices(closingPricesForAllStocks) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me.
One small point: try to keep an eye on indentation, as it will improve readability for other developers.

Comment thread 2-mandatory/3-stocks.js
@@ -48,7 +59,13 @@ function getAveragePrices(closingPricesForAllStocks) {
The price change value should be rounded to 2 decimal places, and should be a number (not a string)
*/
function getPriceChanges(closingPricesForAllStocks) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good as well.
One small comment: Maybe item is not the best variable name here, something like prices will give the reader of the code a bit more information.

Comment thread 2-mandatory/3-stocks.js
@@ -64,7 +81,24 @@ function getPriceChanges(closingPricesForAllStocks) {
The price should be shown with exactly 2 decimal places.
*/
function highestPriceDescriptions(closingPricesForAllStocks, stocks) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good work on this one 👍
In case you're interested, JavaScript also gives us something you might find useful here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/max

* - Return only the frequencies that are radio stations.
*/
// `getStations` goes here
function getStations () {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice solution 👍
For practice, can you re-write this using the filter array method?

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.

done :)

@moneyinthesky

Copy link
Copy Markdown
Contributor

Great job on this coursework @AnnaFYZ

@moneyinthesky moneyinthesky added the reviewed A mentor has reviewed this code label Mar 14, 2023
@AnnaFYZ

AnnaFYZ commented Mar 17, 2023

Copy link
Copy Markdown
Author

Thank you very much for reviewing, recommendations and feedback :)

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

Labels

reviewed A mentor has reviewed this code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants