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

Refactor NullNode and remove NullLeafNode class #73

Merged

Conversation

lu-pinto
Copy link
Contributor

@lu-pinto lu-pinto commented Oct 2, 2024

Remove NullLeafNode and use NullNode instead

Signed-off-by: Luis Pinto <[email protected]>

Remove NullLeafNode and use NullNode instead

Signed-off-by: Luis Pinto <[email protected]>
Copy link

github-actions bot commented Oct 2, 2024

  • I thought about the changelog.

Copy link
Contributor

@matkt matkt left a comment

Choose a reason for hiding this comment

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

LGTM just added small comments

return isLeaf() ? "NullLeafNode" : "NullNode";
}

@SuppressWarnings("rawtypes")
Copy link
Contributor

Choose a reason for hiding this comment

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

for NullNode class maybe better to have the @SuppressWarnings directly for the class ? and not for each method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we make it class wide then other accept methods might not report relevant warnings and they could be useful. I prefer to keep it locally to where we actually need to disable them

return nullLeafNode;
}

public static <T> NullNode<T> nullLeafNode(Optional<T> previousValue) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe better to rename the method to createNullLeafNode or newNullLeafNode

 rename nullLeafNode to newNullLeafNode in NullNode

Signed-off-by: Luis Pinto <[email protected]>
@lu-pinto lu-pinto merged commit e103275 into hyperledger:main Oct 2, 2024
5 checks passed
@lu-pinto lu-pinto deleted the refactor-nullleafnode-and-nullnode branch October 2, 2024 13:32
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.

2 participants