-
Notifications
You must be signed in to change notification settings - Fork 43
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
Ports - Margaret #8
base: master
Are you sure you want to change the base?
Conversation
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.
I want to recognize that you did three separate working implementations with excellent complexity analyses here. Really great job! 🎉 🎉 🎉
It may turn out to be useful for you in interviews to be able to outline (generally in pseudocode, for time) a few different approaches to a problem and explain the time or space complexity for those approaches. I know it took a lot of effort to write all this, but now you have a great example that you can reference when practicing.
And also, you know that you can do it successfully. :)
Checklist
- Clean, working code
- Efficient code
- A detailed explanation of time and space complexity (explains what n stands for, explains why it would be a specific complexity, etc.)
- All test cases for the assignment should be passing.
smaller = array2 | ||
end | ||
|
||
my_hash = Hash.new() #space complexity is Linear / O(m) where m is the smaller of the 2 input arrays |
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.
[nit] I think my_hash = {}
means the same thing and is less verbose?
Also, you should check out Set!
What you're doing here is taking advantage of hash's fast lookup (to get the O(1) rutime for include?
of a hash table). But notice you had to have a placeholder value (the 1
). This is your indication that a Set
might be a more appropriate abstraction, since you can add num
to the set without the placeholder.
(Note that "set" is an abstraction, not a data structure per se -- this means that sets can be implemented with any number of structures that have different performance characteristics, such as hash tables or binary trees. The docs for Ruby lead me to believe that Set
is implemented with a hash under the covers, so it's probably not fundamentally different from what you've implemented.)
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.
FWIW, I don't know if Set
would be considered a "Ruby-provided method", but I wanted you to know about it anyway. :)
# Returns a new array to that contains elements in the intersection of the two input arrays | ||
# Time complexity: ? | ||
# Space complexity: ? | ||
# Time complexity: O(n + m ) where n is the larger array and m is the smaller array, which could just be expressed as O(n). |
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.
Nice. Digressing a little, O(n)
is not the same as O(n + m)
in general, but because of how you've defined it here, n
is always going to be the dominant term. So this analysis absolutely correct! But just be careful because you may encounter other algorithms where O(n + m)
does not equal O(n)
. 😄
It might be better to express this as O(max(n, m))
, where n
is the length of the first input array and m
is the length of the second input array. That puts the "mathy" bit of determining which of the two arrays is largest directly into the complexity calculation.
# Time complexity: ? | ||
# Space complexity: ? | ||
# Time complexity: O(n + m ) where n is the larger array and m is the smaller array, which could just be expressed as O(n). | ||
# Space complexity: O (m) where m is the smaller of the two arrays input into the intersection method. The common elements between the two arrays input are stored in a new array, that could be up to the length of the smaller arrays, m. |
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.
Might be worth mentioning you use space both in my_hash
and common_elements
. They both store m
elements, though, so O(m)
is correct.
You could also use O(min(n, m))
here to avoid saying "the smaller of" when defining m
.
end | ||
|
||
larger.each do |num_1| # time complexity is O(n) where n is larger | ||
if my_hash.include?(num_1) # time complexity is O(1) |
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.
Seeing these comments here tells me you're thinking about all the right things when doing your complexity analysis, so good job! Keep in mind you won't need to write comments like these while working on production systems on the job, but it may be helpful to keep track of during algorithm questions in interviews.
|
||
# ------------------------------------------------------- | ||
# Sorting | ||
# Time complexity: O (m log m) (reduced down from O(m log n) + O (n + log m)) where m is the longer array and n is the shorter array |
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.
I'm having trouble seeing where m log(n) + n + log(m)
came from, but O(m log(m))
is right for this implementation.
Also, when the expression is complex, you can define an intermediate variable, like "the time complexity is O(k log(k))
where k = max(m, n)
and m
and n
are the lengths of the input arrays", if you like the max
/min
suggestions from my other comments.
No description provided.