NW6| Bakhat Begum| Module-JS2| B/week 1 | Week-1|Sprent-1

[BakhatBegum]

Learners, PR Template

Self checklist

• [x] I have committed my files one by one, on purpose, and for a reason
• [x] I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
• [x] I have tested my changes
• [x] My changes follow the style guide
• [x] My changes meet the requirements of this task

I would greatly appreciate your feedback.

[netlify[bot]]

✅ Deploy Preview for cute-gaufre-e4b4e5 ready!

Name	Link
🔨 Latest commit	ab1ef4c2d08fcf0b205e8cf13c168bb44cba5c53
🔍 Latest deploy log	https://app.netlify.com/sites/cute-gaufre-e4b4e5/deploys/65babb408c90bc0008c2a08a
😎 Deploy Preview	https://deploy-preview-139--cute-gaufre-e4b4e5.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[BakhatBegum]

Hello Altom, hope you are well. I have made the necessary changes according to your instructions.

On Thu, 25 Jan 2024 at 22:02, AltomHussain @.***> wrote:

@.**** commented on this pull request.

Great work. Just make sure to have a look at the feedback.

In week-1/fix/median.js https://github.com/CodeYourFuture/Module-JS2/pull/139#discussion_r1466998704 :

+//Start by running the tests for this function +//If you're in the week-1 directory, you can run npm test -- fix to run the tests in the fix directory +

• function calculateMedian(list) {
• const listsToSort= list.sort((a, b) => a - b);
• const middleIndex = Math.floor(listsToSort.length / 2);
• const elementsInMiddle = listsToSort.slice(middleIndex - 1, middleIndex + 1);
• for (let i = 0; i < listsToSort.length; i++){
• if(listsToSort.length % 2 === 0){
• return (elementsInMiddle[0] + elementsInMiddle[1]) / 2;
• }
• else {
• return listsToSort[middleIndex];
• } }

You don't need for loop to check for the array length. if you do this (listsToSort.length % 2 === 0 this will check for array length if it is even or not.

In week-1/implement/dedupe.js https://github.com/CodeYourFuture/Module-JS2/pull/139#discussion_r1467003817 :

@@ -1 +1,16 @@ -function dedupe() {} +function dedupe(item) {

• let arr = [];
• const mySet = new Set();
• for (const elements of item){
• if(!mySet.has(elements)){
• arr.push(elements);

You don't need to use two array variables to do this, instead, you can just use one array to push and check at the same time in your loop.

You can ask me if you don't understand this

In week-1/implement/max.js https://github.com/CodeYourFuture/Module-JS2/pull/139#discussion_r1467006316 :

• arr.push(maxlist[i]);
• }
• else if (typeof maxlist[i] === "string") {
• //parseInt mean to convert the string to a number.
• const numberValue = parseInt(maxlist[i]);
• }
• }
• if(arr.length === 0){
• return -Infinity;
• }
• const maxString= Math.max.apply(null, arr);
• return maxString;
• }
• module.exports = max;
• //console.log(max([2.45,'err' ,3.87, -40,]));

Great work. Bakhat but can you remove the console.log here. You don't need to push any commented code or any console.logs are one of the principle of clean code

In week-1/refactor/find.js https://github.com/CodeYourFuture/Module-JS2/pull/139#discussion_r1467008336 :

@@ -8,6 +8,7 @@ function find(list, target) { } } return -1; + }

module.exports = find;

The comment the top of the file says use for...of loop not for loop. So can you please follow the instructions?

— Reply to this email directly, view it on GitHub https://github.com/CodeYourFuture/Module-JS2/pull/139#pullrequestreview-1844742142, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7AJUMZBNALJBYC35UVTDFLYQLI6VAVCNFSM6AAAAABARFDEMCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNBUG42DEMJUGI . You are receiving this because you authored the thread.Message ID: @.***>