Re: [gobo-eiffel-develop] Binary search tree

[Daniel T. <dan...@gm...>]

Colin Adams wrote:
> You would also need to write some general documentation in the gobodoc format.
>   
Ok.
> I've taken a fairly casual look. Generally it looks good.
>
> The one thing that does stand out is that it does not conform to the
> Gobo styling standards.
>   
I know that the indexing clause does not conform to the Gobo styling 
standards so far. And the class name should be in the same line as the 
class keyword. Those are the two non-conforming things I know. Did you 
find more?
> I'm rather puzzled by this comment:
>
> " Direct instances should not be used in practice as the trees may
> become unbalanced."
>
> The class name (DS_BINARY_SEARCH_TREE) does not dictate balanced
> trees. Nor does there seem to be anything at first glance that
> requires the tree to be balanced.
>
> So perhaps you mean to say that creating direct instances is not
> advisable. (otherwise you could simply mark the class as deferred).
>   
You are right. "Not advisable" is much better.
> In DS_BINARY_SEARCH_TREE_NODE, the assertion tags on the creation
> procedures don't read well to me:
>
> 			reuse_if_no_parent: parent = Void
> 			reuse_if_no_left_child: left_child = Void
> 			reuse_if_no_right_child: right_child = Void
>
> I don't understand what is going on here. Make is not used in the
> class except as a creation procedure, when the assertions are
> trivially True (nor do it seem to be used in the descendants).
>   
This kind of code is actually the reason why I have to check the whole 
code again. The design was not detailed when I started, as I just did 
not know what was needed such that also Red-Black- and AVL-Trees could 
reuse as much code as possible. So at certain positions it is not yet 
good enough.
> Some of the routines are too long to read on the screen.
>   
I don't like that as well. Those are the algorithms that could also be 
implemented as recursive features. I will see what I can do to improve 
that. But I will not make them recursive.
A splitting into more features is also not easy as some features would 
require about 4 or 5 arguments. I was already working on that but 
stopped because the resulting code was not more readable. It could be 
done using attributes. But it would take some time.
Do you think that has to be changed?

Another minor improvement I am going to use is the "Performance: O(...)" 
comment as it is present in some Gobo classes.

Thanks for your comment.