-
Notifications
You must be signed in to change notification settings - Fork 6
Cedar-Bailey #3
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: master
Are you sure you want to change the base?
Cedar-Bailey #3
Conversation
Grabbing this to grade! |
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.
Good work!
I added some notes about your complexity calculations, some debugging code that was left in, and a small bug with your top-K solution. However, this shows adequate demonstration of the material so this is good enough for a Green!
lib/exercises.js
Outdated
// Each subarray will have strings which are anagrams of each other | ||
// Time Complexity: ? | ||
// Space Complexity: ? | ||
// Time Complexity: O(m) m is all of the characters in all of the words |
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.
Usually when doing time complexity like this, variables can't really be assigned for the entire set of input, it only really makes sense to talk about the effect each element of the input has.
So it would be more common to say that m
is the number of characters per word, and then using n
as the number of words in the list, we'd get O(n*m).
However, knowing that the words are English words, we can make a simplifying assumption. English words tend not to get so long, about 5 letters on average. With that knowledge, we can tell that the number of words in the list is going to have much more of an effect than the number of letters in the word, and just say it's 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.
lol I did have this has the time complexity and then talked myself out of it, but thank you for reaffirming my original assumptions.🙃
lib/exercises.js
Outdated
buckets[value].push(key); | ||
} | ||
|
||
console.log(buckets); |
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.
Just a small style thing, but make sure to remove debugging code like this to keep code clean.
lib/exercises.js
Outdated
// fetch the larget frequest bucket first, until reach k | ||
let ans = []; | ||
for (let i = buckets.length - 1; i > 0 && ans.length < k; i--) { | ||
if (buckets[i] !== null) ans.push(...buckets[i]); |
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.
There's a slight bug here that I'm surprised didn't break the tests. To initialize buckets
you added an empty array []
to each bucket. Those empty buckets are getting added to the ans
array too because empty arrays are not !==
to null
. A better check would be buckets[i].length !== 0
. (I guess you could &&
that with a check for null
, but with your current code, none of the elements in buckets
will be null
.)
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.
Note that this bug isn't so major that I'm moving down to Yellow.
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 appreciate your feedback, but a small note about your phrasing "Note that this bug isn't so major that I'm moving down to Yellow." and "this shows adequate demonstration of the material so this is good enough for a Green" are not the most constructive of comments.
lib/exercises.js
Outdated
function validSudoku(table) { | ||
throw new Error("Method hasn't been implemented yet!"); | ||
// Time Complexity: O(n^2) | ||
// Space Complexity: O(n^2) |
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.
These would be correct in the case that we accepted arbitrary board sizes, but since we only accept 3x3 grids, we can actually say that this takes O(1) in both time and space.
Hash Table Practice
Congratulations! You're submitting your assignment!
Comprehension Questions