Re: [cvs] libkdtree++ ChangeLog,1.6,1.7 NEWS,1.2,1.3 README,1.6,1.7 TODO,1.1,1.2

[Martin F K. <kr...@ai...>]

Paul,

Thanks for your changes! Here are some comments and questions:

also sprach Paul Harris (krafft),,, <ph...@cv...> [2004.11.05.0938=
 +0100]:
> +++ README	5 Nov 2004 08:38:09 -0000	1.7
> +It is ok to call insert(value) many times and optimize() at the end, but=
=20
> +every erase() call should be followed with optimize().

Well, not necessarily, but erases could take longer since searches
are involved.

> +++ TODO	5 Nov 2004 08:38:09 -0000	1.2
> +- keep tree balanced in insert() and erase().

this is a nice TODO, but i banged my head over it for a day or two
with no real success. The only way is to optimise after each insert
and erase, but that's going to cost a lot. I prefer to batch-load
the tree, optimise it once, then use it. Furthermore, erasing is
probably not needed IME.

> +++ ChangeLog	5 Nov 2004 08:38:09 -0000	1.7
> +	- Renamed optimise() to optimize().

You want to fight over this? :)

Mh, I would prefer to have this changed back, not because British is
the way to go, but because you are making an API change.

For all I care, let's provide a second inline method optimise which
just calls the other.

> +	- Some code cleanup=20
> removed inlines,

why?

> switched from const_iterator to iterator

why? I make it an effort always to use const unless it's not needed.

I am not sure I see the reason behind your changing of the
constness of the iterators. Please explain!

> +      void insert(_InputIterator __first, _InputIterator __last) {
> +         for ( ; __first !=3D __last; ++__first )
> +            this->insert(*__first);
> +      }

I would prefer these to be done in while loops, I think...

  while (__first !=3D __last) this->insert(*__first++)

but it does not matter really.

> +      insert(iterator __pos, size_type __n, const value_type& __x)
>        {
> +        for ( ; __n > 0; --__n)

On a side note, you are not very consistent about spaces before and
after parentheses. Sometimes they are there, sometimes not. I prefer
them not to be there. Unless you object, let's keep it that way.
I know, this is pedantic. It's not a big thing...

> -        return (_Link_type&) _M_meta->_M_right;=20
> +        return (_Link_type&) _M_header->_M_right;=20

Why did you change most _M_meta for _M_header?

> -      _Link_type _M_meta;
> +      _Link_type _M_header;

Oh, I see. Is there a reason for this change? Or just your
preference? I chose meta because a tree can be seen as a triangle,
and the header node sort of sits *above* that, with its three pointers
going to the three corners.

Thanks for your work!

--=20
Martin F. Krafft                Artificial Intelligence Laboratory
Ph.D. Student                   Department of Information Technology
Email: kr...@ai...          University of Zurich
Tel: +41.(0)44.63-54323         Andreasstrasse 15, Office 2.18
http://ailab.ch/people/krafft   CH-8050 Zurich, Switzerland
=20
Invalid/expired PGP subkeys? Use subkeys.pgp.net as keyserver!
Spamtraps: kra...@ai... kra...@if...
=20
"a mathematician is a device for turning coffee into theorems."
                                                         -- paul erd=F6s