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

London10 -Anu Thapaliya-JS1-Week4#227

Open
anuthapaliy wants to merge 3 commits intoCodeYourFuture:mainfrom
anuthapaliy:main
Open

London10 -Anu Thapaliya-JS1-Week4#227
anuthapaliy wants to merge 3 commits intoCodeYourFuture:mainfrom
anuthapaliy:main

Conversation

@anuthapaliy
Copy link

No description provided.

@anuthapaliy anuthapaliy added the review requested I would like a mentor to review my PR label Apr 1, 2023
function findLongNameThatStartsWithA (names) {
let longName = names.find(names => names.length>7 && names.startsWith("A"));
return longName;
}
Copy link

Choose a reason for hiding this comment

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

There's a simpler way to find words that starts with "A". Instead of startWith method, can you think of accessing first letter using array[index]?


let pairsByIndex; // Complete this statement
let pairsByIndex = pairsByIndexRaw.filter((pairs) => Array.isArray(pairs) && pairs.length ===2);

Copy link

Choose a reason for hiding this comment

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

This works fine. Can you make another attempt using filter() method?

*/

let arr = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];
arr.forEach(array);
Copy link

Choose a reason for hiding this comment

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

To make your codes neater, it's a very good practice to use conditional ternary operator instead of if else chaining. If you do so, you'll have one line of code instead of line 14 to 24

let statement = "I do not like programming";

let result = "";
let result = statement.substring(0, 5).concat(statement.substring(8, statement.length));
Copy link

Choose a reason for hiding this comment

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

A very very tiny mistake! you have one extra space between do and not! can you find out why?


function formatPercentage() {
function formatPercentage(array) {
let input = [];
Copy link

Choose a reason for hiding this comment

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

You don't have to define an empty array. Mapping on array instead of traditional for loop is a neater option.

if(number>100) {
input.push("100%")
}else{
input.push(`${Number(number.toFixed(2))}%`)
Copy link

@migmow migmow Apr 1, 2023

Choose a reason for hiding this comment

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

I recommend using Math.round() instead of toFixed(). Please see the difference. Math.round() always returns an integer and you don't have to convert it to a number

findSafeOxygenLevel(["200%", "-21.5%", "20", "apes", "21.1%"])
).toEqual("21.1%");
});
// test("findSafeOxygenLevel function filters out invalid percentages", () => {
Copy link

@migmow migmow Apr 1, 2023

Choose a reason for hiding this comment

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

hmm, not sure why you have commented out the test

function findSafeOxygenLevel() {}
// Some string methods that might help you here are .replace() and .substring().
function isSafeOxygenLevel(oxygenlevel){
return parseFloat(oxygenlevel)>19.5 && parseFloat(oxygenlevel)<23.5;
Copy link

Choose a reason for hiding this comment

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

This one is intersting! this is not passing the third test(thet you commented:)). To pass this test, you should add a condition or use includes()method to only include strings which have %.
Or forget about parseFloat and use .replace() and .substring() and .map() obviously

//Write your code here
}
function isBerrySafe(berry){
if(berry=== "pink"){
Copy link

@migmow migmow Apr 1, 2023

Choose a reason for hiding this comment

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

If you download prettier extention for JS in your VS code, it makes your codes look nicer! no extra space, etc
Also recommend using eslint js extention

let stayingFamiliesArray = voyagers.filter(likeThisPlanet);
return stayingFamiliesArray;
};

Copy link

Choose a reason for hiding this comment

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

The logic is all correct.
worth making another attemp, only one function getSettlers() and .include() after filter()

let sitInExam = studentAttendance.filter(isEligible);
let examNames = sitInExam.map(onlyNames);
return examNames;
};
Copy link

@migmow migmow Apr 1, 2023

Choose a reason for hiding this comment

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

Same issue here. The logic works absolutly fine. We don't have to have the first two functions. Fewer lines are not always better. But, fewer lines are often better than more lines in terms of readability and maintainability.

//edit code below
if (stringText) {
return stringText;
if (stringText.includes("code")) {
Copy link

Choose a reason for hiding this comment

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

more efficient to pass magicWord instead of "code"

@migmow
Copy link

migmow commented Apr 1, 2023

Well done for finishing JS1 week4! Really good practice of array methods and arrow function. Please let me know if any of the comments isn't clear enough

@migmow migmow added the reviewed A mentor has reviewed this code label Apr 1, 2023
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