London | 25-ITP-Sep | Adnaan Abo | Sprint 1 | Sprint-1#791
London | 25-ITP-Sep | Adnaan Abo | Sprint 1 | Sprint-1#791AdnaanA wants to merge 17 commits intoCodeYourFuture:mainfrom
Conversation
modified: Sprint-1/fix/median.test.js new file: package-lock.json new file: package.json
modified: Sprint-1/implement/dedupe.test.js
modified: Sprint-1/implement/max.test.js
modified: Sprint-1/implement/sum.test.js
modified: Sprint-2/debug/address.js
modified: Sprint-2/implement/contains.test.js
modified: Sprint-2/implement/lookup.test.js
modified: Sprint-2/implement/querystring.test.js
modified: Sprint-2/implement/tally.test.js
deleted: Sprint-2/debug/author.js deleted: Sprint-2/debug/recipe.js deleted: Sprint-2/implement/contains.js deleted: Sprint-2/implement/contains.test.js deleted: Sprint-2/implement/lookup.js deleted: Sprint-2/implement/lookup.test.js deleted: Sprint-2/implement/querystring.js deleted: Sprint-2/implement/querystring.test.js deleted: Sprint-2/implement/tally.js deleted: Sprint-2/implement/tally.test.js deleted: Sprint-2/interpret/invert.js deleted: Sprint-2/package-lock.json deleted: Sprint-2/package.json deleted: Sprint-2/readme.md deleted: Sprint-2/stretch/count-words.js deleted: Sprint-2/stretch/mode.js deleted: Sprint-2/stretch/mode.test.js deleted: Sprint-2/stretch/till.js
modified: Sprint-1/implement/sum.test.js
AdnaanA
added
Needs Review
ckirby19
added
Review in progress
ckirby19
left a comment
ckirby19
left a comment
There was a problem hiding this comment.
Very, very nice work! All your solutions make sense and your code is very clean. I have just added a few comments for extension
| numericList.sort((a, b) => a - b); | ||
| const middleIndex = Math.floor(numericList.length / 2); | ||
| let median; | ||
| if (numericList.length % 2 === 0) { |
There was a problem hiding this comment.
This solution is already perfect, but just as an extension, could you show me how you could express this if...else statement using a ternary operator?
There was a problem hiding this comment.
Your right we can express the if...else statement using a ternary operator here is how our median calculation will look like using the ternary operator:
numericList.sort((a, b) => a - b);
const middleIndex = Math.floor(numericList.length / 2);
const median = numericList.length % 2 === 0
? (numericList[middleIndex - 1] + numericList[middleIndex]) / 2
: numericList[middleIndex];
There was a problem hiding this comment.
Exactly! Great stuff. In my experience with javascript/typescript, the ternary operator is often preferred over if..else, if there are no "else if" conditions
| @@ -1,4 +1,24 @@ | |||
| function findMax(elements) { | |||
| if (elements.length === 0) { | |||
There was a problem hiding this comment.
You may have heard of the DRY (Don't Repeat Yourself) principle in coding, which emphasises reducing the amount of code that is repeated (such as the same if (elements.length === 0) code you have here and below).
Although these if statements represent different cases (an empty array vs. one with all elements that are non-numeric), they can both be covered just with the if statement on line 8
There was a problem hiding this comment.
Removed the first if statement since since the if statement at line 8 covered both cases. and i have updated the code accordingly
ckirby19
added
Reviewed
AdnaanA
added
the
Needs Review
|
setting this as complete, all comments seem resolved |
mjpeet
added
Complete
4 participants
Self checklist
Change list:
In sum.js I have implemented a function that sums only the numeric elements of an array, ignoring non-numerical values, and handles empty arrays, negatives, and decimals, with tests in sum.test.js.
In max.js I have implemented a function that finds the largest numeric element in an array while ignoring non-numeric values and supporting empty arrays, negatives, and decimals, tested in max.test.js.
In dedupe.js I have implemented a function that removes duplicate values from arrays of numbers or strings, preserving the first occurrence and handling empty arrays, with tests in dedupe.test.js.
In calculateMedian.js I have implemented a function that calculates the median of numeric elements from sorted or unsorted arrays of odd or even length, filtering out non-numeric values, tested in median.test.js.
In includes.js I have refactored the function that checks if a target value exists in an array using a for...of loop, supporting empty arrays, duplicates, and nulls, with tests in includes.test.js.