Data data structures #6

thamaranth · 2017-02-06T17:15:21Z

check out this data

added several stack functions with tests

renamed test files and added in Queue function

…data-structures into day2DataStructures

* WIP for PQueue * WIP for PQueue * set WIP * WIP for Set * Linked List

Day2 data structures

added work for several structures

finished linkedList

finished double linked list

mantinone

Sorry I ran out of time! I can review your tests later if you like.

mantinone · 2017-02-06T17:29:57Z

src/doubleLinkedList.js

+  insertAfter(item, data) {
+    const newNode = new Node(data)
+
+    if (this.find(item) === this.tail {


There was a syntax error here that had to be fixed before tests could be run. (the if statement is not enclosed)

Also, you left a couple console logs in here.

mantinone · 2017-02-06T17:33:49Z

src/stack.js

+    return count
+  }
+
+}


Looks good! I would try using your own length function instead of the built in length property, just for the heck of it.

mantinone · 2017-02-06T17:37:30Z

src/set.js

+  forEach(func) {
+    this.elements = this.elements.map(func)
+    return this.elements
+  }


I feel like this should be implemented differently for learning purposes. You have essentially used Map to implement Map.

mantinone · 2017-02-06T17:43:25Z

src/queue.js

+    return count
+  }
+
+}


mantinone · 2017-02-06T17:44:31Z

src/set.js

+      return index == self.indexOf(element);
+    })
+    return unique
+  }


This is a pretty clever way of doing it.

mantinone · 2017-02-06T17:50:19Z

src/pQueue.js

+    }
+    return null
+  }
+


You're repeating this "Find the max" "Find the min" code a few times here. It would be a good idea to break these out into helper functions so they don't have to be repeated, and it helps make the code more easily readable, too.

mantinone · 2017-02-06T17:55:11Z

src/linkedList.js

+    return this.find(item) !== -1
+  }
+
+}


Looks good.

mantinone · 2017-02-06T17:55:20Z

src/doubleLinkedList.js

+  contains(item) {
+    return this.find(item) !== -1
+  }
+}


jaredatron

wow this was a long review.

jaredatron · 2017-02-06T19:04:59Z

.gitignore

@@ -1 +1,3 @@
 node_modules/
+lib/
+.gitignore


I'm not even sure you can ignore the .gitignore from within the .gitignore itself. That seems paradoxical :p

jaredatron · 2017-02-06T19:05:13Z

README.md

+_Provide a brief, high-level overview of what the final product (artifact) of this goal is. Include any relevant resources or dependencies here._

-Base repository for the [Core Data Structures](https://github.com/GuildCrafts/web-development-js/issues/127) goal.
+Write tests and implementations for common data structures.


why change the readme?

jaredatron · 2017-02-06T19:06:01Z

lib/pQueue.js

@@ -0,0 +1,156 @@
+'use strict';


it looks like you've checked in a transpiled version of one of your source files. These generally are not committed.

jaredatron · 2017-02-06T19:07:08Z

.gitignore

@@ -1 +1,3 @@
 node_modules/
+lib/


lib is in your git ignore but you are still tracking files within lib. git rm -df lib/* and then commit the removal of these files from tracking

jaredatron · 2017-02-06T19:08:05Z

spec/doubleLinkedList_test.js

+      expect(doubleLinkedList.contains('foo'))
+        .to.be.true
+      expect(doubleLinkedList.contains('tap'))
+        .to.equal(false)


.to.be.false ?

jaredatron · 2017-02-06T20:25:57Z

src/set.js

+      return true
+    }
+    return false
+


remove whitespace

jaredatron · 2017-02-06T20:26:28Z

src/set.js

+
+  }
+
+  contains(value) {


contains(value) { return this.elements.indexOf(value) !== -1 }

jaredatron · 2017-02-06T20:28:16Z

src/set.js

+
+  remove(value) {
+    if (this.elements.indexOf(value) > 0) {
+      return this.elements.splice(this.elements.indexOf(value),1)


remove(value) { if (this.contains(value)) return false this.elements = this.elements.filter(member => member !== value) return true }

jaredatron · 2017-02-06T20:30:15Z

src/set.js

+  }
+
+  forEach(func) {
+    this.elements = this.elements.map(func)


this would be better

forEach(iterator) { this.elements.forEach(iterator) return this }

This would be better for your learning

forEach(iterator) { for (let i = this.length - 1; i >= 0; i--) { iterator(this.elements[i], i) } return this }

jaredatron · 2017-02-06T20:31:00Z

src/set.js

+  }
+
+  size() {
+    let count = 0


for this exercise you should be tracking size so you don't have to rely on the elements array to calculate it

tannerwelsh and others added 28 commits December 31, 2016 11:09

Skeleton for tests & implementations using babel, mocha, chai

6cc3efc

Fix link

bf34f68

Update README.md

212bed1

added several stack functions with tests

7ee84a6

finished Stack functions

5d67965

Merge pull request #1 from breyana/day1DataStructures

541a81a

added several stack functions with tests

renamed test files and added in Queue function

30c2723

added in Deonna's suggestions for formatting and cleaner code

79194e5

refactored Stack

f179393

WIP for PQueue

71ff38d

Merge remote-tracking branch 'origin' into day2DataStructures

cde3914

Merge pull request #2 from breyana/day2DataStructures

2b3390c

renamed test files and added in Queue function

Merge branch 'day2DataStructures' of https://github.com/breyana/core-…

2956c24

…data-structures into day2DataStructures

WIP for PQueue

3a605c2

Merge remote-tracking branch 'origin' into day2DataStructures

5ee76c8

Merge branch 'day2DataStructures' of https://github.com/breyana/core-…

377d117

…data-structures into day2DataStructures

WIP for PQueue

333d6c2

set WIP

c77a653

WIP for Set

794fdf0

Day2 data structures (#3)

6b6e0bc

* WIP for PQueue * WIP for PQueue * set WIP * WIP for Set * Linked List

Merge pull request #1 from thamaranth/day2DataStructures

7cd855d

Day2 data structures

fixes merge

d1a473e

added work for several structures

1591475

Merge pull request #2 from thamaranth/DBLLL

15a3aa0

added work for several structures

finished linkedList

c7cee07

Merge pull request #3 from thamaranth/SNGL

a2e8c96

finished linkedList

finished double linked list

b3d837f

Merge pull request #4 from thamaranth/DBLLL

ce4d291

finished double linked list

mantinone reviewed Feb 6, 2017

View reviewed changes

jaredatron reviewed Feb 6, 2017

View reviewed changes

Data data structures #6

Are you sure you want to change the base?

Data data structures #6

Uh oh!

Conversation

thamaranth commented Feb 6, 2017

Uh oh!

mantinone left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredatron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants