Repository navigation
NW| 26-Jul-SDC | Ahmad Hmedan | Sprint 1 | Analyse and Refactor Functions - #226
AhmadHmedann wants to merge 12 commits into
Conversation
AhmadHmedann
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
The complexity analysis is spot on. The code looks good.
Could you ensure all the code is consistently formatted?
| if (secondSet.has(item)) | ||
| //O(1) on Ave | ||
| commonSet.add(item); //O(1) On Ave |
There was a problem hiding this comment.
Check out this coding style article for guidance on if statements.
| product:int = 1 | ||
| for num in input_numbers: | ||
| total+=num | ||
| product*=num |
There was a problem hiding this comment.
The spacing around the assignment operator is not consistent.
| # common_items: List[ItemType] = [] | ||
| # for i in first_sequence: | ||
| # for j in second_sequence: | ||
| # if i == j and i not in common_items: | ||
| # common_items.append(i) | ||
| # return common_items |
There was a problem hiding this comment.
What's the complexity of the original code? (The Python implementation is different from the JS implementation)
There was a problem hiding this comment.
original
Time Complexity: O(n * m * k), worst case O(n^3)
Two nested loops compare the sequences, and checking
"i not in common_items" can require scanning the result list.
new
Time Complexity : O(n+m)
There was a problem hiding this comment.
Why enlarged the text? It looks like 'yelling".
There was a problem hiding this comment.
Sorry about that. I didn’t realise that adding # would make the line render as a large heading. It looked small while I was typing it, so I didn’t notice.
There was a problem hiding this comment.
I know you didn't meant it.
I always use the "Preview" feature to check what I typed on GitHub.
There was a problem hiding this comment.
Thank you for the feedback. From now on, I’ll use the Preview feature to check my comments before posting them.
cjyuan
added
Reviewed
AhmadHmedann
added
Needs Review
cjyuan
added
Reviewed
AhmadHmedann
added
Needs Review
cjyuan
added
Complete
|
Why leave the "Changelist" section empty? |
2 participants
Learners, PR Template
Self checklist
Task code
CYF-1173
Changelist