Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Ports - Margaret #8

wants to merge 1 commit into from

Conversation

msfinnan
Copy link

No description provided.

Copy link

@mmcknett mmcknett left a 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
Copy link

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.)

Copy link

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).
Copy link

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.
Copy link

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)
Copy link

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants