London | 25-SDC-Nov | Aida Eslamimoghadam | Sprint 1 | Analyse and refactor functions#141
London | 25-SDC-Nov | Aida Eslamimoghadam | Sprint 1 | Analyse and refactor functions#141aydaeslami wants to merge 10 commits intoCodeYourFuture:mainfrom
Conversation
cjyuan
left a comment
There was a problem hiding this comment.
Code is good.
Can you also show the time complexity of the original implementation?
| * Time Complexity: | ||
| * Space Complexity: | ||
| * ANSWER: O(n + m) | ||
| Because the second array is converted into a Set once, and the first array is looped through once. | ||
|
|
There was a problem hiding this comment.
I think "Time Complexity" here refers to the time complexity of the original implementation, and "Optimal Time Complexity" refers to the time complexity of the refactored version. (Otherwise they will always have the same complexity)
| if i in second_sequence_set and i not in common_items: | ||
| common_items.append(i) |
There was a problem hiding this comment.
Can also consider using a built-in set operation for better performance and simpler code.
There was a problem hiding this comment.
Thanks for your feedback @cjyuan !I’ve made the suggested changes, including using the built-in set intersection and updating the complexity comments. Please let me know if anything else needs improvement.
cjyuan
left a comment
There was a problem hiding this comment.
Can you also show the time complexity of the original implementation (in each file)?
| Time Complexity: | ||
| Space Complexity: | ||
| Optimal time complexity: | ||
| Time Complexity: O(n + m) process both sequences |
There was a problem hiding this comment.
The complexity of the original implementation is not O(n+m).
Self checklist
Changelist
Refactored functions to reduce time complexity by removing nested loops and using sets for faster lookups, improving overall performance.