[Betternestedset-talk] Questions, RC for 0.1?

Jean-Christophe Michel
2006-12-13T03:04:20Z

Hi Krishna,

Le 13 d?c. 06, ? 02:03, Krishna Dole a ?crit :
> I''ve been committing a bunch of changes over the last few days,
and
> we''re down to one ticket on trac now, so I think we should start
> considering the current code as a release candidate for 0.1.

Sure, you''ve done a considerable amount of work. Many thanks !

> First of all, I think that #move_to_child_of should insert the new
> child to the right of existing children. Here''s why:
> - consistent with the creation of new records (to the right of
> existing ones)
> - consistent with #add_child
> - consistent with default ordering for things like threaded posts,
> where new items are added after existing ones
> - if the parent has a large number of children relative to the size of
> the tree, performance will be better, because fewer rows need to be
> updated
>
> What do you think? I know this will be a compatibility issue, so if we
> are going to do it  perhaps we should wait until after 0.1.

why not. Let''s change this before 0.1.

> The second thing I want to talk about would also cause compatibility
> problems. ;)  It concerns #children_count. The present name of the
> method is highly misleading, because it actually counts _all_ children
> of the node, so presently node.children.size != node.children_count.
> If we are going to change it to #all_children_count, I would favor
> changing it to the much more natural #count_all_children instead. (I
> would also change #leaves_count to #count_leaves). Thoughts?

I copied behaviour as much as possible from acts_as_tree. I agree the
meaning is not clear.
If we choose to break acts_as_tree similarity, we can rename it.
Or do we need to open a better_acts_as_tree project ? ;-)

> The third issue concerns concurrency problems. Prior to revision 25,
> the following code would corrupt the left/right indexes:
>
>     c1, c2, c3 = Category.create, Category.create, Category.create
>     c1.move_to_right_of(c3)
>     c2.save  # c2 is stale, and overwrites the correct lft/rgt values
>
> My solution was to override the #update method in ActiveRecord to
> ignore the lft/rgt columns (the code is at the bottom of
> better_nested_set.rb). I don''t know if this is a good way to do
> things, so I''m hoping that someone else can take a look at the
code
> and see if it is sane.

Don''t you think we''ll solve the problem by defining
   attr_reader left_column_name
   or maybe, define #{left_column}=() {} method
so none of left, right and parent could be set from code (only changed
from sql)
DId you try this already ?

Jean-Christophe Michel
--
symetrie.com

Better Nested Set for rails: