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

Map for data structures #88

Merged
merged 14 commits into from Aug 15, 2014
Merged

Map for data structures #88

merged 14 commits into from Aug 15, 2014

Conversation

joshwget
Copy link
Contributor

No description provided.

@eush77
Copy link
Contributor

eush77 commented Aug 12, 2014

Why did you call it map exactly? IMO this is forEach.

@felipernb
Copy link
Owner

I agree with @eush77, map usually returns an Array with the transformed results while forEach doesn't (and that's what you're doing there)

@@ -102,4 +102,18 @@ HashTable.prototype._increaseCapacity = function () {
}
};

HashTable.prototype.map = function (fn) {
var applyFunction = function (linkedList) {
linkedList.map( function (elem) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linter is complaining about this space before function (to test it, run make locally)

@joshwget
Copy link
Contributor Author

I had the same thought about the name, but I saw map was already used in linked lists so I just kept it for consistency. My local build didn't catch any of the syntax problems, but I think the problem was coming from running "make" as root. Thanks for the comments though, I'll make these changes soon.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 5f7b84b on joshuacurl:map into fd04111 on felipernb:master.

@felipernb
Copy link
Owner

Yeah, you're right about the LinkedList map, I'll change it to forEach :)

felipernb added a commit that referenced this pull request Aug 14, 2014
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling eed6f49 on joshuacurl:map into a4b8d5b on felipernb:master.

@joshwget
Copy link
Contributor Author

The best way I could think of to iterate through a heap in order was to copy the elements and call extract until it was empty, like heap sort.

felipernb added a commit that referenced this pull request Aug 15, 2014
Map for data structures
@felipernb felipernb merged commit e794d88 into felipernb:master Aug 15, 2014
@felipernb
Copy link
Owner

Thanks for the contribution @joshuacurl!

I've merged it and changed somethings by myself as well:

  1. f92635e - Having JSON.parse(JSON.stringify(...)) changes the items stored inside the array and we don't want that. The side-effects can be pretty bad if you're using regular objects (they lose their prototype and are become new objects with different references), so I'm just creating a new array and passing all the existing objects to it.
  2. ecfb649 The forEach for a Hash Table should receive the key and the value.

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.

None yet

4 participants