Skip to content

Conversation

@weihanglo
Copy link
Contributor

  • Fix Issue: left/right tree should reconnect to new node
  • Test: unmark .travis.yml test
  • Lint files
  • Update README.markdown
Copy link
Member

@kelvinlauKL kelvinlauKL left a comment

Choose a reason for hiding this comment

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

Most changes look good. I'd like to discuss more on the remove method changes.

if let left = left {
replacement = left.maximum()
} else if let right = right {
if let right = right {
Copy link
Member

Choose a reason for hiding this comment

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

What's the significance of handling the right child before the left?

Copy link
Contributor Author

@weihanglo weihanglo Feb 5, 2017

Choose a reason for hiding this comment

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

The previous version before 942e3b1 use successor to replace the node with 2 children. The unit tests are also based on this logic. I am kind of lazy to modify all the test cases 😵.

Copy link
Member

Choose a reason for hiding this comment

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

Alright cool. I think this checks out. Merging!

@kelvinlauKL
Copy link
Member

Thanks @weihanglo!

@kelvinlauKL kelvinlauKL merged commit c5f482c into kodecocodes:master Feb 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants