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

London Class 10 - Andrius Isin - JS1 - Week 4 #232

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AndriusIsin
Copy link

No description provided.

@AndriusIsin AndriusIsin added the review requested I would like a mentor to review my PR label Mar 29, 2023
@AndriusIsin AndriusIsin requested a review from llamojha March 29, 2023 12:15
let longNameThatStartsWithA = names.find(getlongNameThatStartsWithA);
// let longNameThatStartsWithA = names.find((element) => {
// return element.length > 7 && element.startsWith("A");
// });

Copy link

Choose a reason for hiding this comment

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

I really like the "startsWith" method you used. Nice! but can you think of another way of checking the second condition without using any method?

let student = students[indexes[0]];
let mentor = mentors[indexes[1]];
return [student, mentor];
if (indexes !== null) {
Copy link

Choose a reason for hiding this comment

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

Your condition works fine, so well done for that! but the aim of this exercise is using some() method on pairsByIndex array. Could you think of using some() instead of line 19.

console.log("Fizz");
} else if (number % 5 === 0) {
console.log("Buzz");
} else {
Copy link

@migmow migmow Mar 31, 2023

Choose a reason for hiding this comment

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

Please read the instruction again, Andrius. There are 3 conditions. You have successfully done first and second condetions but could you add the last condition as well and print "FizzBuzz" when needed.
Also, it's a good practice if in the future you could use ternary operator instead of if else chaining.

let arr = str.split("");
arr[0] = arr[0].toUpperCase();
return arr.join("");
}

Copy link

Choose a reason for hiding this comment

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

Nice one indeed!

.replace(/day/g, "night")
.replace(/great/g, "brilliant")
.replace(/10/g, "100000");
console.log(result);
Copy link

Choose a reason for hiding this comment

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

Is there any specific reason that you used regular expression?

@@ -33,7 +38,8 @@ Write a function that:
- Returns a new array containing the same elements, but without the element at the passed index.
*/

function remove() {
function remove(array, index) {
return array.filter((element, i) => i !== index);
Copy link

Choose a reason for hiding this comment

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

This could pass your tests but it isn't very efficient. Imagine if you have a lenghthy array, looping through isn't efficient here. I recommend using methods like slice and concat

number = 100;
}
return `${Math.round(number * 100) / 100}%`;
});
Copy link

Choose a reason for hiding this comment

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

This function doesn't pass the test and returns undefined.

function formatPercentage() {
function formatPercentage(numbers) {
return numbers.map((number) => {
if (number > 100) {
Copy link

Choose a reason for hiding this comment

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

The instruction is saying if number >= 100 then return "100%". Please amend your if statement

function formatPercentage(numbers) {
return numbers.map((number) => {
if (number > 100) {
number = 100;
Copy link

Choose a reason for hiding this comment

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

This bit isn't right. Can you think why? You are looping through array of numbers and then reassigning 100 to each element in return.

return "Bush is safe to eat from";
} else {
return "Toxic! Leave bush alone!";
}
Copy link

@migmow migmow Mar 31, 2023

Choose a reason for hiding this comment

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

Nice one!
Two things to do if you're interested in learning more!
1- using some() and every() method together as the instruction says
2- when you have if else statement you can remove else, and just return what is supposed to be in else. Please research about it. It cleans up your code!

function getSettlers() {}
function getSettlers(voyagers) {
return voyagers.filter(
(family) => family.startsWith("A") && family.includes("family")
Copy link

Choose a reason for hiding this comment

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

Again startsWith is really handy here, but there's another simple way to get the first index of word and see if it is equal to "A"

return students
.filter((student) => student[1] >= 8)
.map((student) => student[0]);
}

Copy link

Choose a reason for hiding this comment

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

Well done for using filter and map

// Implement the function body
return locations
.filter((location) => location.includes(transportMode))
.map((location) => location[0]);
}
Copy link

Choose a reason for hiding this comment

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

This is fine Andrius, you have already defined a funcition above called getLocationName, can you think how you can use this function in line 133?

@migmow migmow added the reviewed A mentor has reviewed this code label Mar 31, 2023
@migmow
Copy link

migmow commented Mar 31, 2023

Hey Andrius, really well done for finishing this coursework, it wasn't mandatory though! Please read the comments and I'm happy to discuss them if it's not clear

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
review requested I would like a mentor to review my PR reviewed A mentor has reviewed this code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants