Repository navigation
London | 26-SDC-Jul | AYK | Sprint 1 | Analyse and Refactor Functions #237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,26 @@ | ||
| /** | ||
| * Finds common items between two arrays. | ||
| * | ||
| * Time Complexity: | ||
| * Space Complexity: | ||
| * Optimal Time Complexity: | ||
| * Time Complexity: O(n2) | ||
| * Space Complexity: O(n) | ||
| * Optimal Time Complexity:O(n) | ||
| * | ||
| * @param {Array} firstArray - First array to compare | ||
| * @param {Array} secondArray - Second array to compare | ||
| * @returns {Array} Array containing unique common items | ||
| */ | ||
| export const findCommonItems = (firstArray, secondArray) => [ | ||
| ...new Set(firstArray.filter((item) => secondArray.includes(item))), | ||
| ]; | ||
|
|
||
| //refactor code | ||
| export function findCommonItems (firstArray, secondArray){ | ||
|
|
||
| const firstSet = new Set(firstArray); | ||
| const secondSet = new Set(secondArray); | ||
|
|
||
| const commonSet = firstSet.intersection(secondSet); | ||
|
|
||
|
|
||
| return [...commonSet]; | ||
| } | ||
|
|
||
|
|
||
| // This refactor code is better because it avoid to check every single item from two arrays for filter items & searching that each items includes in the second array . | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your complexity analysis is correct if the length of both arrays are Also, your explanation does not quite explain how you derived the optimal complexity. Could you elaborate? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,16 @@ | ||
| /** | ||
| * Find if there is a pair of numbers that sum to a given target value. | ||
| * | ||
| * Time Complexity: | ||
| * Space Complexity: | ||
| * Optimal Time Complexity: | ||
| * Time Complexity:O(n2) | ||
| * Space Complexity:O(1) | ||
| * Optimal Time Complexity:O(n) | ||
| * | ||
| * @param {Array<number>} numbers - Array of numbers to search through | ||
| * @param {number} target - Target sum to find | ||
| * @returns {boolean} True if pair exists, false otherwise | ||
| */ | ||
|
|
||
| /* | ||
| export function hasPairWithSum(numbers, target) { | ||
| for (let i = 0; i < numbers.length; i++) { | ||
| for (let j = i + 1; j < numbers.length; j++) { | ||
|
|
@@ -19,3 +21,27 @@ export function hasPairWithSum(numbers, target) { | |
| } | ||
| return false; | ||
| } | ||
| */ | ||
|
|
||
| //refactor code | ||
|
|
||
| export function hasPairWithSum(numbers,target){ | ||
| const seen = new Set(); | ||
|
|
||
| for (const num of numbers) | ||
| { | ||
| const needed = target-num; | ||
| if (seen.has(needed)) | ||
| { | ||
| return true; | ||
| } | ||
| else | ||
| { | ||
| seen.add(num); | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
Comment on lines
+28
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
|
|
||
| // The refactor code is better because it avoid nested loop but it create set so space complexity will be o(n). | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Saying "avoid nested loops" does not fully explain why the refactored implementation is better than the original. Instead, you could compare their time complexities to clearly explain the performance improvement. On separate note, could you elaborate how you determined the complexity of the refactored implementation is |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| /** | ||
| * Remove duplicate values from a sequence, preserving the order of the first occurrence of each value. | ||
| * | ||
| * Time Complexity: | ||
| * Space Complexity: | ||
| * Optimal Time Complexity: | ||
| * Time Complexity:O(n2) | ||
| * Space Complexity: O(n) | ||
| * Optimal Time Complexity: O(n) | ||
| * | ||
| * @param {Array} inputSequence - Sequence to remove duplicates from | ||
| * @returns {Array} New sequence with duplicates removed | ||
| */ | ||
|
|
||
| /* | ||
| export function removeDuplicates(inputSequence) { | ||
| const uniqueItems = []; | ||
|
|
||
|
|
@@ -34,3 +36,15 @@ export function removeDuplicates(inputSequence) { | |
|
|
||
| return uniqueItems; | ||
| } | ||
| */ | ||
|
|
||
| //refactor code | ||
| export function removeDuplicates(inputSequence) | ||
| { | ||
| const outputSequence = [...new Set(inputSequence)]; | ||
| return outputSequence; | ||
| } | ||
|
|
||
|
|
||
| //The refactor code is better because it avoid the nested for loop operation .... | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you also rephrase this statement to explain how you determine the complexity of the refactored code is |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spacing around
=is inconsistent.