go-art icon indicating copy to clipboard operation
go-art copied to clipboard

Fix #5: Store complete key in leaf nodes.

Open nickpalmer opened this issue 10 years ago • 4 comments

nickpalmer avatar Oct 05 '15 00:10 nickpalmer

Heya!

Thanks for your PR, I appreciate you taking the time to do it! I have a few comments for you on your work; I think there are some improvements that can be made such that we don't change the method signature of NewArtNode and so that *ArtNode.Key() returns the same type as we originally inserted into the key.

Let me know your thoughts! Would love to have this in the lib :)

-Kelly

kellydunn avatar Oct 07 '15 05:10 kellydunn

Let me know if you have further comments.

nickpalmer avatar Oct 26 '15 06:10 nickpalmer

Thanks again! @nickpalmer :)

I left a few more comments, it looks like there was a temp file that was commited, and I had a small change suggestion to how insertHelper was being invoked.

Aside from that, I think I'd like to see a test that might assert that a newly created LeafNode does in fact have the Key it was intended to have originally. That would be awesome!

(Thanks again for all your hard work!)

kellydunn avatar Oct 26 '15 07:10 kellydunn

I already added a test that asserts the newly inserted key matches the expected key. See art_tree_test.go:230-233

nickpalmer avatar May 24 '16 20:05 nickpalmer