London | May-2025 | Reza Jahanimir | sprint1 #531
Conversation
Reza-Jahanimir
added
📅 Sprint 1
Sprint-1/fix/median.js
Outdated
|
|
||
| // If the length of the sorted list is even, add 0.5 to the middle number | ||
| if (sortedList.length % 2 === 0) { | ||
| return (sortedList[middleIndex - 1] + 0.5); |
There was a problem hiding this comment.
Can you think of a test case where this logic will fail? May be add it to median.test.js as well :)
There was a problem hiding this comment.
Rather than thinking through the math, I took the lazy route.
There was a problem hiding this comment.
amazing! now just uncomment the correct implementation so that all tests are green again
| // Iterate through the array | ||
| // If the item is not in the set, add it to both the set and the result array | ||
| for (const item of array) { | ||
| if (!seen.has(item)) { |
There was a problem hiding this comment.
Good work! Can you explain why the if condition is necessary here?
There was a problem hiding this comment.
// if will compare the item with the item inside seen and as seen is a set it will not allow duplicate values
// when we loop through the array it will check if the item is already in the set or not
// if it is the if condition will be false and it will not add the item to the result array
// if it is not in the set it will add the item to the set and also to the result array
// so at the end we will have a result array with unique values only
| if (numericValues.length === 0) return -Infinity; | ||
|
|
||
| // Return the largest number in the sorted list | ||
| return Math.max(...numericValues); |
There was a problem hiding this comment.
good code, but the comment has an inaccuracy. Please review the comment above this line
There was a problem hiding this comment.
your are right I am not working with a sorted array, i am simply filtering numeric values and getting the max
Sprint-1/implement/sum.js
Outdated
| // If the sorted list is empty, return null | ||
| if (numericValues.length === 0) return 0; | ||
|
|
||
| // Return the largest number inMath.sum(...numericValues); the sorted list |
There was a problem hiding this comment.
the comment is not related to the code below?
| @@ -1,951 +1,1029 @@ | |||
| -10 | |||
There was a problem hiding this comment.
Great job solving the stretch. But any reason to push the input changes as part of the MR?
There was a problem hiding this comment.
I thought it is mandatory. Sorry my mistake
mhmdk
added
Reviewed
Reza-Jahanimir
added
Needs Review
mhmdk
added
Complete
Learners, PR Template
Self checklist
Changelist
I have tried my best to complete the tasks and write logical testing for them
Questions
Ask any questions you have for your reviewer.