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

Cedar-Bailey #3

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
22 changes: 19 additions & 3 deletions lib/heapsort.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
const { MinHeap } = require('./minheap');
// This method uses a heap to sort an array.
// Time Complexity: ?
// Space Complexity: ?
// Time Complexity: O(n log n)
// Space Complexity: O(n + log n) because of the length of the array and the heap storage-> O(n)
Comment on lines +3 to +4

Choose a reason for hiding this comment

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

✨ Great. Since sorting using a heap reduces down to building up a heap of n items one-by-one (each taking O(log n)), then pulling them back out again (again taking O(log n) for each of n items), we end up with a time complexity of O(2n log n) → O(n log n). While for the space, we do need to worry about the O(log n) space consume during each add and remove, but they aren't cumulative (each is consumed only during the call to add or remove). However, the internal store for the MinHeap does grow with the size of the input list. So the maximum space would be O(n + log n) → O(n), since n is a larger term than log n.

Note that a fully in-place solution (O(1) space complexity) would require both avoiding the recursive calls, as well as working directly with the originally provided list (no internal store).

function heapsort(list) {
throw new Error('Method not implemented yet...');
let minHeap = new MinHeap();

for (let num of list) {
minHeap.add(num);
}

let i = 0;
let currentMin;

while (!minHeap.isEmpty()) {
currentMin = minHeap.remove();
list[i] = currentMin;
i++;
}

return list;
};
Comment on lines +15 to 22

Choose a reason for hiding this comment

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

Note the since this isn't a fully in-place solution (the MinHeap has a O(n) internal store), we don't necessarily need to modify the passed in list. The tests are written to check the return value, so we could unpack the heap into a new result list to avoid mutating the input.

    const result = [];
    while (!minHeap.isEmpty()) {
        result.push(minHeap.remove())
    }

    return result;


module.exports = heapsort;
58 changes: 45 additions & 13 deletions lib/minheap.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,26 @@ class MinHeap {
}

// This method adds a HeapNode instance to the heap
// Time Complexity: ?
// Space Complexity: ?
// Time Complexity: O(log n)
// Space Complexity: O(log n) because of recursion
Comment on lines +14 to +15

Choose a reason for hiding this comment

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

✨ You're exactly right that the time complexity is O(log n), which is due to the maximum number of swaps we might need to perform to get the node to the proper location, and the height of a heap is log n.

👀 You would be correct that the space complexity is also O(log n) when using a recursive approach due to stack growth. However your heapUp uses an iterative approach, avoiding the height-dependent stack growth. Instead, the space complexity is simply O(1), constant.

add(key, value = key) {
throw new Error("Method not implemented yet...");
let newNode = new HeapNode(key, value);

Choose a reason for hiding this comment

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

Prefer const unless we need to reassign the variable.

    const newNode = new HeapNode(key, value);

this.store.push(newNode);
this.heapUp(this.store.length - 1);
}

// This method removes and returns an element from the heap
// maintaining the heap structure
// Time Complexity: ?
// Space Complexity: ?
// Time Complexity: O(log n)
// Space Complexity: O(log n) because of recursion
Comment on lines +24 to +25

Choose a reason for hiding this comment

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

✨ Nice. You're right that the log space complexity for remove is due to the recursive heapDown implementation. We could achieve O(1) space complexity if we used an iterative approach.

remove() {
throw new Error("Method not implemented yet...");
if (this.isEmpty()) return;

this.swap(0, this.store.length - 1);
let minValue = this.store.pop()

Choose a reason for hiding this comment

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

Prefer const unless we need to reassign the variable.

    const minValue = this.store.pop()

this.heapDown(0);

return minValue.value;
}


Expand All @@ -38,26 +46,50 @@ class MinHeap {
}

// This method returns true if the heap is empty
// Time complexity: ?
// Space complexity: ?
// Time complexity: O(1)
// Space complexity: O(1)
Comment on lines +49 to +50

Choose a reason for hiding this comment

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

isEmpty() {
throw new Error("Method not implemented yet...");
return this.store.length === 0;
}

// This helper method takes an index and
// moves it up the heap, if it is less than it's parent node.
// It could be **very** helpful for the add method.
// Time complexity: ?
// Space complexity: ?
// Time complexity: O(log n)
// Space complexity: O(1)
Comment on lines +58 to +59

Choose a reason for hiding this comment

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

✨ Exactly. This is where the main cost of add comes from, which is why add actually has a space complexity of O(1), constant.

heapUp(index) {
throw new Error("Method not implemented yet...");
let index1 = index;

Choose a reason for hiding this comment

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

👀 There's no need to copy the value of index into a local index1. Even though we'll be re-assigning to index, it won't affect the value outside the function (reassigning simply changes what value the local index variable refers to). So we could work with just index and parentIndex throughout.

let parentIndex = Math.floor((index - 1)/2);

while (parentIndex >= 0 && this.store[parentIndex].key > this.store[index1].key) {

Choose a reason for hiding this comment

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

✨ Nice iterative approach, which reduces the space complexity without increasing the time complexity.

this.swap(index1, parentIndex);
index1 = parentIndex;
parentIndex = Math.floor((index1 - 1)/2);
}
Comment on lines +64 to +68

Choose a reason for hiding this comment

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

Here's another approach that would let us perform the parentIndex calculation only once (we can get rid of the once before the start of the loop). It also allows us to simplify the while condition a bit.

    while (index > 0) {
      const parentIndex = Math.floor((index - 1)/2);
      
      if (this.store[parentIndex].key < this.store[index].key) { break; }

      this.swap(index, parentIndex);
      index = parentIndex;
    }

}

// This helper method takes an index and
// moves it up the heap if it's smaller
// than it's parent node.
heapDown(index) {
throw new Error("Method not implemented yet...");
let leftChildIndex = (2 * index) + 1;
let rightChildIndex = (2 * index) + 2;
Comment on lines +75 to +76

Choose a reason for hiding this comment

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

👀 These could both be const

let nextRootIndex;

if (leftChildIndex < this.store.length && rightChildIndex < this.store.length) {
if (this.store[leftChildIndex].key > this.store[rightChildIndex].key) {
nextRootIndex = rightChildIndex;
} else {
nextRootIndex = leftChildIndex;
}
} else {
nextRootIndex = leftChildIndex;
}
Comment on lines +77 to +87

Choose a reason for hiding this comment

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

If we initialize next to be the more common outcome of the conditions, we can reduce the complexity of the conditional clauses. Further, we can observe that the right child being in bounds implies the left must be as well and omit that from the condition.

    let nextRootIndex = leftChildIndex;

    if (rightChildIndex < this.store.length) {
      if (this.store[leftChildIndex].key > this.store[rightChildIndex].key) {
        nextRootIndex = rightChildIndex;
      }
    }


if (nextRootIndex < this.store.length && this.store[nextRootIndex].key < this.store[index].key) {
this.swap(index, nextRootIndex);
this.heapDown(nextRootIndex);
}
}

// If you want a swap method... you're welcome
Expand Down
3 changes: 2 additions & 1 deletion test/heapsort.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const expect = require('chai').expect;
const heapsort = require('../lib/heapsort');

describe.skip("heapsort", function() {
describe("heapsort", function() {
it("sorts an empty array", function() {
// Arrange
const list = [];
Expand Down