Abdelrazak Younes <[EMAIL PROTECTED]> writes:
| Lars Gullik Bjønnes a écrit :
| > Abdelrazak Younes <[EMAIL PROTECTED]> writes:
| > | I have put convenience function so as to minimize the changes
| > needed
| > | in the code that uses ParagraphList. But there are some minimal (IMHO)
| > | change. The interface is simpler so a lot of code could be simplified
| > | with this new class.
| > By using a slightly different approach we can minimize this even
| > futher, and do it in steps. IMHO you patch as it stands now has too
| > many small changes in too many places.
|
| Well, the only small changes is that "erase" or "insert" arguments are
| position instead of iterator. If you want, I can add two convenience
| functions that will accept iterators instead. This is easy but not
| very efficient because I would have then to look for them in the
| vector of iterators.
Care about the interface first, not if it is efficient right away or
not.
| Then there are also some use of (iterator+position) that I had to fix.
| These are only valid if iterator is a vector::iterator. In my patch it
| is a list::iterator underneath so this doesn't work. This is why I had
| to write "it++" instead of "it+1" in some cases.
In some of those cases you should have used boost::next.
| > IMHO first step is to make ParagraphList a proper class that
| > _contains_ a (private) vector. Methods should be added to this
| > ParagraphList so that what we use from the vector is covered. Nothing
| > more. No tricks with using etc. should be used.
|
| I thought that's what I did... except of course that I already use a
| list inside.
Exactly. One step at a time. Make sure everything works after every
step. I do not want us to break anything, not for an iota.
| > When that is done, we can as a first approach, decide that _only_ the
| > internal implementation of ParagraphList should change. Interface
| > stays put. (Now we can add the list and the change the vector to hold
| > iterators (f.ex.) instead. The vector is then just a cache, and can be
| > invalitated at any time, it will just be rebuilt.)
|
| With my patch you are already there and it works :-) (well, there are
| still some crashes with Cut&Paste but I am confident I can solve that).
Apart from the fact that I am not too comfortable with your patch.
(Too many things at the same time.)
| IMHO the changes I propose to the code are very minimal and I think I
| have covered every potential interface problem. If it was my code I
| would have gone much further... for example by cleaning up the
| interface call altogether.
Do it in steps...
| Of course this matter is 1.5 only if you accept it.
I will need some work, but sure it is the right way forward.
--
Lgb
