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

Glasgow 6 - Alena Lazareva - JS Core 1 - Week 3 - #238

Open
lazar-eva wants to merge 4 commits into
CodeYourFuture:mainfrom
lazar-eva:main
Open

Glasgow 6 - Alena Lazareva - JS Core 1 - Week 3 #238
lazar-eva wants to merge 4 commits into
CodeYourFuture:mainfrom
lazar-eva:main

Conversation

@lazar-eva

Copy link
Copy Markdown

No description provided.

// TODO
const statements = [];
cities.forEach(city => {
statements.push('The temperature in '.concat(city, ' is ',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Prefer using string interpolation instead of the concat method https://syllabus.codeyourfuture.io/js-core-1/week-1/lesson#string-concatenation

function getTemperatureReport(cities) {
// TODO
const statements = [];
cities.forEach(city => {

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 believe this exercise was meant to be solved by simple iteration as forEach is only introduced during week 4. On the other hand if you are already using items from week 4 then prefer solving this exercise using map: https://syllabus.codeyourfuture.io/js-core-1/week-4/lesson#map

*/
function potentialHeadlines(allArticleTitles) {
// TODO
const smallArticle = allArticleTitles.filter(article => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if it's a one liner feel free to immediately return the value without assigning it to a variable

fewestWordTitle = article;
}
}
return fewestWordTitle;

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 a good solution with simple use of an iterated array. You can of course also do it using forEach as well. If you are happy to investigate other solutions using inline functions (like you did with filter above), you can have a read of how the reduce function works that can be used to implement things like miniimums, maximums or sums: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce

function headlinesWithNumbers(allArticleTitles) {
let articleWithNums = [];
for (let i = 0; i < allArticleTitles.length; i++) {
if (/\d/.test(allArticleTitles[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.

Nice use of regular expressions


function headlinesWithNumbers(allArticleTitles) {
let articleWithNums = [];
for (let i = 0; i < allArticleTitles.length; 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.

Similarly you already used forEach for other solutions, feel free to do that here as well

allArticleTitles.forEach(article => {
sumOfAllChars += article.length;
});
return Math.round(sumOfAllChars / allArticleTitles.length);

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 yet again a good solution. If you are interested the reduce function can also be used here to calculate the result

test("should return the average number of characters in a headline", () => {
expect(averageNumberOfCharacters(ARTICLE_TITLES)).toEqual(65);
});
}); No newline at end of file

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's customary to have a newline at every file ending when handling code. Usually your IDE can be set up in a way to enforce this and most do it by default

Comment thread 2-mandatory/3-stocks.js
Solve the smaller problems, and then build those solutions back up to solve the larger problem.
Functions can help with this!
*/
// function getAveragePrices(closingPricesForAllStocks) {

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's usually good practive to remove commented code from PRs

@sztupy sztupy 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.

Nice answers and a good display of the ability on handling arrays and values inside. Also a good show of all the various ways to handle things - which is expected in a learning scenario.

On the other hand in a professional environment consistency is usually preferred:

  • Prefer only using the for (element of array) type of iterator, except in case the index value is required as well
  • Similarly toFixed(2) is usually a better way to convert to two decimal places than using Math.round
  • String interpolation is usually a better way than either using + or using the concat function for strings

It was also nice seeing regular expressions to be used in the code!

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