Abdelrazak Younes <[EMAIL PROTECTED]> writes: > Lars Gullik Bjønnes a écrit : > > I agree with the criticism against these functions. They should both > > do the same thing: move iterators forward and backwards. > > I don't understand. These two operators have nothing to do one with the > other. "operator+" takes a difference_type argument and "operator-" > takes an iterator. I think you are mis leaded by the fact operator- is > defined inside the class instead of: > > difference_type operator-(iterator it1, iterator it2) const { > return std::distance(it1, it2); > }
(Non-member functions can't be const :)) Actually, I don't think that this is really a criticism of your code (operator-) but rather of the existing LyX sources that assume the use of a Random Access Iterator. If we really want our code to be container-agnostic then we should be using std::distance directly. operator- implies that the iterators are Random Access iterators with O(1) behaviour. > (You have the vector, and can thus get O(1) operator+, but not if you > > use the list::iterator.) > > IMHO, you want the butter and the money of the butter. Is that the same as "On ne peut pas manger l'omelette sans casse' les oeufs"? ("You can't have your cake and eat it" in English.) > > Copy is the wrong name for this. In the other containers this is > > called 'assign'. > > 'copy' means more to me that 'assign' but OK. If you're making an STL-like container, use STL terminology... > > I also think that it_vector really should be a RandomAccessList, tuned > > for only working with std::list as the main container and std::vector > > as its 'iterator cache' > > I need more explanation. For me, if you restrain the use to operator[] > and operator() access ParagraphList would be plenty fast. I would really > like to keep compatibility with the inner container (in this case > std::list). I think that Lars is going too far too fast. (Funnt, since he insisted that you break your original patch down into smaller chunks of which this is the first.) If profiling tells us that we need O(1) operator[] access to the underlying std:list container, then so be it. First, however, I think that we should try out your class as it is now. Angus