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

Fix insert and remove method #28

Merged
merged 3 commits into from
Jun 18, 2024
Merged

Fix insert and remove method #28

merged 3 commits into from
Jun 18, 2024

Conversation

arriqaaq
Copy link
Contributor

Description

This pull request (PR) addresses two critical issues:

1. Bug in remove Method

  • Issue: The remove method previously had a significant bug where it would remove all children matching a particular prefix without considering the type of the child node.
  • Fix: The method has been updated to include a check on the type of child node before performing the removal. This ensures that only the intended nodes are removed, maintaining the integrity of the tree structure.

2. Enhancement in insert Method

  • Issue: The insert method did not check if the node type was full before attempting to add children. This could lead to failures when the tree is shrunk to the last key and a new key is inserted without prior growth.
  • Fix: The method now first checks if the node type is full. If it is, the node is grown accordingly before adding any children. This preemptive growth ensures that the tree can accommodate new keys without failing.

These changes improve the reliability and robustness of the tree's remove and insert operations.

Copy link

@emmanuel-keller emmanuel-keller left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

@phughk phughk left a comment

Choose a reason for hiding this comment

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

Nice one

@tobiemh tobiemh merged commit 06dfa21 into main Jun 18, 2024
1 check passed
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.

4 participants