wolkykim / qlibc

qLibc is a simple and yet powerful C library providing generic data structures and algorithms.
http://wolkykim.github.io/qlibc
Other
983 stars 167 forks source link

A problem (?) in qtreetbl #63

Closed darmar-lt closed 2 years ago

darmar-lt commented 7 years ago

Hello A few months ago I developed a wrapper for some of qlibc containers to be called from Fortran language (here). Thanks to the developers of qlibc!

Recently one of the user, frazar, performed tests with qtreetbl container. He found, that an invariant 4 "If a node is red, then both its children are black." (Wikipedia) is broken in his test case. Original message of frazar can be found here. I repeated the same test with qlibc in my fork.

Does the rule "If a node is red, then both its children are black" should be always enforced in the realized algorithm?

darmar-lt commented 6 years ago

I have read a bit about LLRB trees. Now I can answer to my question: Yes, the rule "If a node is red, then both its children are black" should be always satisfied.

Then I have tried to find in the code where is the problem. Sedgewick in his slides ( slides ) presented two variations of "insert" function: a) with colorFlip on the way down (page 42) (2-3-4 variation of tree) b) with colorFlip on the way up (page 45) (2-3 variation of tree)

In "qtreetbl.c" we have (a).

On page of Kohler ( here ) is mentioned: "The delete implementation, however, only works for 2–3 trees. If you implement the default variant of insert and the only variant of delete, your tree won’t work."

Exactly this happened in the current implementation of "qtreetbl.c".

I consideren two possible solutions: 1) Switch to 2-3 variation of the tree, i.e. to move colorFlip ("flip_color" function in "qtreetbl.c") to the end of "put_obj" function. 2) To add additional check in the "delete" function ("remove_obj" function in "qtreetbl.c").

Both solutions seems to pass the tests. Which is better? Solution (2) I commited into my fork development branch.

wolkykim commented 5 years ago

can you send a pull request for the update? and do you have a unit test that tests this deletion case? if you have that too that would be really awesome.

wolkykim commented 2 years ago

this has been addressed with https://github.com/wolkykim/qlibc/pull/66