NW | 25-ITP-Sep| Ahmad Hmedan | Sprint 1 | Sprint 1 Coursework#833
NW | 25-ITP-Sep| Ahmad Hmedan | Sprint 1 | Sprint 1 Coursework#833AhmadHmedann wants to merge 14 commits intoCodeYourFuture:mainfrom
Conversation
…n array,otherwise return null..........Filtered out non-numeric element .......Added a check for emty numeric arrays ....Cloned the array to prevent mutation(avoid side effects)......Sorted numbers in ascending order.......Calculated the median for both even and odd length
…e array is empty and update the function's code to handles this scenario
…andle all test scenarios the scenarios are \n-empty array\n -Single elemnt array \n -Mixed positive and negative numbers\n-All negative numbers\n-Decimal values\n-Array with mixed (numeric/non-numeric)values \n- Array with only non-numeric values
…all test scenarios the scenarios are \n-empty array\n -Single elemnt array \n -Mixed positive and negative numbers\n-All negative numbers\n-Decimal values\n-Array with mixed (numeric/non-numeric)values \n- Array with only non-numeric value
AhmadHmedann
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Can you check if any of this general feedback can help you further improve your code?
https://github.com/CodeYourFuture/Module-Data-Groups/blob/sprint-1-feedback/Sprint-1/feedback.md
Doing so can help reviewers speed up the review process. Thanks.
cjyuan
added
Reviewed
AhmadHmedann
added
Needs Review
Sprint-1/fix/median.js
Outdated
| return (numericList[middleIndex] + numericList[middleIndex - 1]) / 2; | ||
| } else { | ||
| const middleIndex = Math.floor(numericList.length / 2); | ||
| const median = numericList.splice(middleIndex, 1)[0]; |
There was a problem hiding this comment.
There is a much more efficient syntax to access an element at a particular index without deleting any element from the array.
Sprint-1/implement/dedupe.js
Outdated
| for (let i = 1; i < elementsClone.length; i++) { | ||
| for (let j = i; j > 0; j--) { | ||
| if (elementsClone[i] === elementsClone[j - 1]) { | ||
| elementsClone.splice(i, 1); | ||
| i--; // because I deleted one element from my array | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
You approach works. Here are some suggestions for you to consider:
-
Deleting an element from an array is a relatively costly operation. A more efficient alternative is to append an element to a new array only if the element does not exist in the array.
-
Most modern programming languages offer a built-in class that implements the Set data structure. It is optimized to keeps only unique items. Do look up how to use the
Setclass in JS.
I am not sure what your "it" in "... it must be an array" refers to. For that code to work, |
cjyuan
added
Reviewed
AhmadHmedann
added
Needs Review
cjyuan
added
Complete
3 participants
Learners, PR Template
Self checklist
Changelist
I have implemented the sum, max, dedupe, and median functions, and written comprehensive tests for each. I also refactored the includes function.
Questions
const numericList = list.filter(
(item) => typeof item === "number" && !isNaN(item)
);
I know how to use it and why, but I don’t know how it works. When it says it must be an array, does that mean the structure should be an array like const x = [...]? and const x = 23 is not an array, right?