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

London -10/Anu Thapaliya/JS-Core 1-Week3/exercises and mandatory done - #252

Open
anuthapaliy wants to merge 1 commit into
CodeYourFuture:mainfrom
anuthapaliy:main
Open

London -10/Anu Thapaliya/JS-Core 1-Week3/exercises and mandatory done#252
anuthapaliy wants to merge 1 commit into
CodeYourFuture:mainfrom
anuthapaliy:main

Conversation

@anuthapaliy

Copy link
Copy Markdown

No description provided.

@AnnaFYZ AnnaFYZ 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! :) really good job


function getTemperatureReport(cities) {
// TODO
forecast = []

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

it looks like the result is going to be "underfind". You need ro use special wording to declare a variable.

function getTemperatureReport(cities) {
// TODO
forecast = []
for(city of cities){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same is here, "city" is not declared yet


}
return titleCharacters;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

in my opinion it is perfect code :)

function headlinesWithNumbers(allArticleTitles) {
// TODO
function findingNumber (str){
return /[0-9]/.test(str);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i like this solution! well done 👍

return titleWithNumbers;

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think it is better have function separately, not one inside another, so you can use them any time.

Comment thread 2-mandatory/3-stocks.js

for(let types of closingPricesForAllStocks){
let total = 0;
let average = 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.

maybe it would be better to declare "average" variable outside the "for" function, as it is not used inside it, but outside

Comment thread 2-mandatory/3-stocks.js
let firstDayPrice = types[0];
let lastDayPrice = types[types.length-1];

changesInPrice.push(Math.round((lastDayPrice - firstDayPrice)*100)/100);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why do we need here to multiply and the then divide by 100?

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.

Multiplying by 100, using Math.round, and then dividing by 100 is one way to round a number to 2 decimal places in JavaScript. This page goes over a couple of ways you can round a number to 2 decimal places: https://linuxhint.com/round-number-to-2-decimal-places-javascript/

@@ -12,8 +13,19 @@
*/

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.

This implementation looks good 👍
As mentioned below, you may just need to check that you're using let or const when declaring a new variable. Even though the code works without it, it's a good habit to get into - otherwise we'll run into problems when we work with larger code bases 😄

Implement the function below, which returns the title with the fewest words.
(you can assume words will always be seperated by a space)
*/
function titleWithFewestWords(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.

This looks almost perfect to me! A couple of very minor points:

  • Are you happy with the variable name shortHeadlines? Maybe think about what value this variable is holding - can you think of a better name for it?
  • It's a good idea to keep an eye on indentation and spacing - this will make it easier for other developers to read your code. Can you see any indentation in this function that could be improved? 😄

function headlinesWithNumbers(allArticleTitles){
let titleWithNumbers = [];
for (let headline of allArticleTitles) {
if (findingNumber(headline)=== true){

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.

When you have an if statement that looks like this: if (findingNumber(headline)=== true){, you can usually re-write it to be if (findingNumber(headline)){.
Can you think of why that works?

Comment thread 2-mandatory/3-stocks.js
@@ -34,8 +34,25 @@ const CLOSING_PRICES_LAST_5_DAYS_FOR_ALL_STOCKS = [
Functions can help with this!
*/

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 - great job 👍

@moneyinthesky

Copy link
Copy Markdown
Contributor

Great job on this coursework @anuthapaliy

@moneyinthesky moneyinthesky added the reviewed A mentor has reviewed this code label Mar 16, 2023
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.

3 participants