Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 156 additions & 0 deletions spec/doubleLinkedList_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
import chai, { expect } from 'chai'
import chaiChange from 'chai-change'
import DoubleLinkedList from '../src/doubleLinkedList'

chai.use(chaiChange)

describe('Double Linked List', () => {
'use strict'

it('exists', () => {
expect(DoubleLinkedList).to.be.a('function')
})

describe('insert()', () => {
it('Inserts a node (with the provided data) to the tail of the list', () => {
const doubleLinkedList = new DoubleLinkedList()

expect(() => doubleLinkedList.insert('foo'))
.to.alter(() => doubleLinkedList.count, { from: 0, to: 1 })
expect(doubleLinkedList.getHeadNode().data)

Choose a reason for hiding this comment

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

Instead of relying on the internal data representation in Node, consider adding an accessor method:

doubleLinkedList.getHeadNode().getData()

If you later choose to change how data is represented in the node, since you have programmed to the interface, instead of the implementation, your code (both in this test and in the list implementation) will not need to change.

Copy link
Owner Author

Choose a reason for hiding this comment

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

hmm, can I get an example?

Choose a reason for hiding this comment

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

class Node {
  getData() {
    return this.data
  }
}

Everywhere you currently use node.data would need to be updated to node.getData(). Now, regardless of any change you make to the internal structure of a Node, so long as you preserve the getData functionality, your code will not break.

Choose a reason for hiding this comment

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

More on this: let's say I change Node's data field to be named lkhsklkhdsfglkh. I would then change getData to return that field. Anything in our codebase that was referencing node.data would break. Any where that was referencing getData would not.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ahhhh, I see. That makes a lot more sense now.

.to.equal('foo')
expect(() => doubleLinkedList.insert('boo'))
.to.alter(() => doubleLinkedList.count, { from: 1, to: 2 })
expect(doubleLinkedList.getTailNode().data)
.to.equal('boo')
})
})

describe('insertFirst()', () => {
it('Inserts a node (with the provided data) to the head of the list', () => {
const doubleLinkedList = new DoubleLinkedList()

expect(() => doubleLinkedList.insertFirst('foo'))
.to.alter(() => doubleLinkedList.count, { from: 0, to: 1 })
expect(() => doubleLinkedList.insertFirst('boo'))
.to.alter(() => doubleLinkedList.count, { from: 1, to: 2 })
console.log(doubleLinkedList)
expect(doubleLinkedList.getHeadNode().data)
.to.equal('boo')
expect(doubleLinkedList.getTailNode().data)
.to.equal('foo')
})
})
describe('contains()', () => {
it('Determines whether or not the list contains the provided data', () => {
const doubleLinkedList = new DoubleLinkedList()
doubleLinkedList.insertFirst('foo')
doubleLinkedList.insertFirst('boo')

expect(doubleLinkedList.contains('foo'))

Choose a reason for hiding this comment

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

I try to create tests that test only one assertion - the true and false cases are two different tests that are not necessarily related. If you somehow manage to "break" the true path, this test would count as failed, even though the second assertion may not be failing... Isolating assertions in individual tests helps you granularly test, and explicitly identify broken functionality with the test suite.

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

describe('find()', () => {
it('Determines whether or not the list contains the provided data', () => {
const doubleLinkedList = new DoubleLinkedList()
doubleLinkedList.insertFirst('foo')
doubleLinkedList.insertFirst('boo')
doubleLinkedList.insertFirst('shoo')
doubleLinkedList.insertFirst('test')
console.log(doubleLinkedList)

expect(doubleLinkedList.find('shoo').data)
.to.equal('shoo')
expect(doubleLinkedList.find('tap'))
.to.equal(-1)
})
})

describe('clear()', () => {
it('Clears the list of all nodes/data', () => {
const doubleLinkedList = new DoubleLinkedList()
doubleLinkedList.insertFirst('foo')
doubleLinkedList.insertFirst('boo')
doubleLinkedList.insert('shoo')

expect(doubleLinkedList.clear())

Choose a reason for hiding this comment

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

This is not a meaningful test - a function without an explicit return statement returns undefined in JavaScript. This test will always pass, even if the clear() function is broken. You have a number of tests like this that should be removed (and perhaps replaced by tests that will fail if the function is broken).

.to.be.undefined
})
})

describe('removeFirst()', () => {
it('Removes the head node from the list', () => {
const doubleLinkedList = new DoubleLinkedList()
doubleLinkedList.insertFirst('foo')
doubleLinkedList.insertFirst('boo')
doubleLinkedList.insert('shoo')

expect(doubleLinkedList.removeFirst())
.to.be.undefined
expect(doubleLinkedList.getHeadNode().data)
.to.equal('foo')
expect(doubleLinkedList.removeFirst())
.to.be.undefined
expect(doubleLinkedList.getHeadNode().data)
.to.equal('shoo')
})
})

describe('remove()', () => {
it('Removes the tail node from the list', () => {
const doubleLinkedList = new DoubleLinkedList()
doubleLinkedList.insertFirst('foo')
doubleLinkedList.insertFirst('boo')
doubleLinkedList.insert('shoo')

expect(doubleLinkedList.remove())
.to.be.undefined
expect(doubleLinkedList.getTailNode().data)
.to.equal('foo')
expect(doubleLinkedList.remove())
.to.be.undefined
expect(doubleLinkedList.getTailNode().data)
.to.equal('boo')
})
})

describe('insertAfter()', () => {
it('Inserts a node (with data) after the first node containing "(item)"', () => {
const doubleLinkedList = new DoubleLinkedList()
doubleLinkedList.insertFirst('foo')
doubleLinkedList.insertFirst('boo')
doubleLinkedList.insert('shoo')

expect(doubleLinkedList.insertAfter('shoo', 'moo'))
.to.be.undefined
expect(doubleLinkedList.getTailNode().data)
.to.equal('moo')
expect(doubleLinkedList.insertAfter('moo', 'doo'))
.to.be.undefined
expect(doubleLinkedList.getTailNode().data)
.to.equal('doo')
})
})

describe.only('insertBefore()', () => {
it('Inserts a node (with data) before the first node containing (item)', () => {
const doubleLinkedList = new DoubleLinkedList()
doubleLinkedList.insertFirst('foo')
doubleLinkedList.insertFirst('boo')
doubleLinkedList.insert('shoo')

expect(doubleLinkedList.insertBefore('shoo', 'moo'))
.to.be.undefined
expect(doubleLinkedList.getTailNode().data)
.to.equal('shoo')
expect(doubleLinkedList.insertBefore('shoo', 'doo'))
.to.be.undefined
expect(doubleLinkedList.getTailNode().data)
.to.equal('shoo')
})
})
})
2 changes: 1 addition & 1 deletion spec/linkedList_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ describe('Linked List', () => {
})
})

describe.only('insertBefore()', () => {
describe('insertBefore()', () => {
it('Inserts a node (with data) before the first node containing (item)', () => {
const linkedList = new LinkedList()
linkedList.insertFirst('foo')
Expand Down
150 changes: 150 additions & 0 deletions src/doubleLinkedList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
'use strict'

class Node {
constructor(data) {
this.data = data
this.next = null
this.prev = null
}
}

export default class DoubleLinkedList {
constructor() {
this.count = 0
this.head = null
this.tail = null
}

getHeadNode() {
return this.head
}

getTailNode() {
return this.tail
}


find(item) {
let currentNode = this.head
while (currentNode) {

Choose a reason for hiding this comment

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

Indentation should be consistently left aligned at the same level of a given block.

if (currentNode.data === item) {
return currentNode
} else {
currentNode = currentNode.next
}
}
return -1
}

clear() {
this.head = null
this.tail = null
this.count = 0
}

insert(item) {
const newNode = new Node(item)

if (this.head === null) {
this.head = newNode
} else {
this.tail.next = newNode
newNode.prev = this.tail
}
this.tail = newNode
this.count++

return newNode

Choose a reason for hiding this comment

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

Should we be returning a Node from the insert function?

Copy link
Owner Author

Choose a reason for hiding this comment

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

should we have any return?

Choose a reason for hiding this comment

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

Not according to the spec.

}

insertFirst(item) {
const newNode = new Node(item)

if (this.head) {
this.head.prev = newNode
newNode.next = this.head
} else {
this.tail = newNode
}

this.head = newNode
this.count++

return newNode

}

insertBefore(item, data) {
const newNode = new Node(data)
newNode.next = this.find(item)

if (newNode.next === this.head) {
this.head.prev = newNode
this.head = newNode
} else {
let currentNode = this.head
while (currentNode.next !== newNode.next) {
currentNode = currentNode.next

Choose a reason for hiding this comment

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

Indentation

Copy link
Owner Author

Choose a reason for hiding this comment

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

how should it look?

Choose a reason for hiding this comment

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

currentNode = currentNode.next

is in the while block , so should be indented under while:

} else {
  let currentNode = this.head

  while( currentNode.next !== newNode.next ) {
    currentNode = currentNode.next
  }
}

}
newNode.prev = currentNode
currentNode.next = newNode
}
this.count++

}

insertAfter(item, data) {
const newNode = new Node(data)

if (this.find(item) === this.tail {
newNode.prev = this.tail

Choose a reason for hiding this comment

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

Indentation.

Copy link
Owner Author

Choose a reason for hiding this comment

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

same question as above

Choose a reason for hiding this comment

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

same answer :P

this.tail = newNode
}
newNode.next = this.find(item).next
newNode.prev = this.find(item)
this.find(item).next = newNode
this.count++

}

remove() {
if (this.count === 1) {
this.clear()
} else {
let currentNode = this.getHeadNode()

Choose a reason for hiding this comment

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

I don't understand this remove code? Couldn't you just use tail's prev to do this?

const newTail = this.tail.prev

this.tail.prev = null
newTail.next = null

this.tail = newTail

while (currentNode.next.next) {
currentNode = currentNode.next
}
currentNode.next = null
this.count--
this.tail = currentNode
}

}

removeFirst() {
if (this.count === 1) {
this.clear()
} else {
this.head = this.getHeadNode().next
this.count--

Choose a reason for hiding this comment

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

You would also need to reset the new head's prev reference to null.

}

}

isEmpty() {
if(!this.head){

Choose a reason for hiding this comment

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

I prefer explicit comparisons, instead of relying on "falsiness" of null/undefined:

return this.head === undefined

Also, isEmpty should be returning a boolean true or false. Your implementation returns "truthy" or "falsey".

Copy link
Owner Author

Choose a reason for hiding this comment

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

can you tell me more about the difference?

Choose a reason for hiding this comment

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

I suggest you check out https://github.com/getify/You-Dont-Know-JS/blob/master/types%20%26%20grammar/ch4.md to understand coercion and its implications for truthyness and falsyness.

Copy link
Owner Author

Choose a reason for hiding this comment

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

thank you

return null
} else {
return this.data
}
}

size() {
return this.count
}

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