[PUSHED] Re: [PATCH] Convert SV_DECL_PTRARR_SORT (8)

David Tardon dtardon at redhat.com
Thu Jul 12 22:06:32 PDT 2012


Hi,

On Thu, Jul 12, 2012 at 02:16:50PM +0200, Michael Stahl wrote:
> On 11/07/12 13:41, Noel Grandin wrote:
> > Hi
> > 
> > These patches convert various places to use normal STL containers.
> > 
> > Particular attention should be paid to patch 0007, since it introduces a 
> > new template container class sorted_vector
> > 
> > These patches all pass a full "make check"
> 
> thanks, pushed.
> 
> patch #4 introduced a memory leak, fixed it by using boost::ptr_set.
> 
> patch #7 had weird copyright headers but i just saw your other mail that
> that was an error so i've taken the liberty to exchange these for the
> standard one.
> 
> i've also changed the sorted_vector to inherit private from std::vector,
> as calling vector::insert on it would be a bug anyway, so it should not
> be in the public interface.

Let me add a few nitpicks :-) The only way to ensure that the invariant
of the class holds (that is a fancy way to say that the vector remains
sorted) is to only allow immutable access to its elements. Which means
that

using std::vector<Value>::begin;
using std::vector<Value>::end;
using std::vector<Value>::operator[];
iterator find( const Value& x );

are wrong, because they return either non-const iterator or reference to
an element, thus allowing outside code to modify it. Further

typedef typename std::vector<Value>::iterator  iterator;

is not necessary and

std::pair<iterator,bool> insert( const Value& x )

should be changed to

std::pair<const_iterator,bool> insert( const Value& x )

D.


More information about the LibreOffice mailing list