Re: [Libreoffice] Help on removing a DECLARE_LIST()

Hi Joseph,

On Tue, 2010-12-07 at 06:37 -0800, Joseph Powers wrote:
> Kohei,
>
>
> Help!
>
>
> From calc/sc/inc/rangelst.hxx:
>
>
> typedef ScRange* ScRangePtr;
> typedef ::std::vector< ScRangePtr > ScRangeListBase;
> class SC_DLLPUBLIC ScRangeList : public ScRangeListBase, public
> SvRefBase { ... }
>
>
> I'm changing the definition of ScRangeListBase from a DECLARE_LIST()
> to a ::std:vector<>. For the most part the change is going well;
> however, we have a method in the class named "Join" that looks like
> it's trying to merge two lists together.

You are close, but not exactly.  ScRangeList::Join() adds a new range to
the list of ranges but compacts the list such that, if two ranges can be
represented by one larger range it merges the two into one.

For instance, imagine the following ranges:

+--------+------+
|        |      |
|    A   |  B   |
|        |      |
+--------+------+

where A was already in the list and B is being added to the list via
Join() call.

Because A and B can be represented by a larger single range, Join()
merges the two into a single range upon addition of B.

Note that in this example the two ranges are adjacent, but they can be
overlapping.  I can can't draw overlapping ranges using ascii art.

> My issues are related to nOldPos:
>
>
> 1) I might need to write code to do the GetPos(); this just returns
> the Index to the address given. This is easy and not a big concern.

Ok.

> 2) The Remove( nOldPos ) can be replaced with an erase( begin() +
> nOldPos ); however, at this point I believe that it's removing the
> object because it's in both lists (they have the same address). I'm
> thinking that destroying one instance will destroy the 2nd which would
> make this code incorrect. (I know, it's been working for years; I just
> don't see how).

The existing code is correct because that Remove() call simply removes
the pointer value stored in the list, and the removal of the pointer
itself doesn't remove the object that the pointer points to.  So, when
you remove a pointer stored in the list, and you no longer need the
object that resides at the address pointed by the pointer, then you need
to manually delete that object.  This is what the existing code does.

> 3) Why is this recursive? This my have something to do with ScRange
> acting like a ScRangeList; which is my next task to find out why.

This needs to be recursive because merging of two ranges may trigger
their merged range to become mergeable with another range stored in the
list.  For example, consider the following:

+--------+------+
|        |      |
|    A   |  B   |
|        |      |
+--------+------+
|               |
|       C       |
|               |
+---------------+

where range B is being added, and A and C are already in the list.
Addition of B causes A and B to get merged into one, which in turn
causes this merged A:B to be merged with C.  This can go on recursively
until no two ranges in the list can be merged.  This is what Join()
does.

> 4) All the comments being in German isn't helping... 

Indeed.  We still have lots of untranslated german comments in calc's
code.

> 5) I'm thinking the Seek() is related to updating the position for the
> List iterator. vector<> uses a different iterator process so this can
> be removed; however, do I need to do something to update the
> vector<>'s iterator?

I'm pretty sure that we can ignore this call when we switch from
DECLARE_LIST to vector.

It's probably helpful to point out that the actual List implementation
that DECLARE_LIST declares lives here

http://opengrok.go-oo.org/xref/libs-gui/tools/inc/tools/list.hxx#42
(class List)

which uses most of its gut from class Container

http://opengrok.go-oo.org/xref/libs-gui/tools/inc/tools/contnr.hxx#48

Class Container appears to divide the internal storage into multiple
blocks, and the Seek() call seems to update the current block position
that contains specified index (probably for speed reasons).

http://opengrok.go-oo.org/xref/libs-gui/tools/source/memtools/contnr.cxx#1509

So, I'm willing to bet that there is nothing we need to do here to
replace this call.

I hope this helps.  If not, let me know.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida@novell.com>