Re: About erasing iterators (ohierarchyholder.cxx, package module)

On 27/08/2013 09:04, Jan Holesovsky wrote:
>      289 void OHierarchyElement_Impl::RemoveElement( const ::rtl::Reference<
> OHierarchyElement_Impl>&  aRef )
>      290 {
>      291     {
>      292         ::osl::MutexGuard aGuard( m_aMutex );
>      293         OHierarchyElementList_Impl::iterator aIter =
> m_aChildren.begin();
>      294         const OHierarchyElementList_Impl::const_iterator aEnd =
> m_aChildren.end();
>      295         while (aIter != aEnd)
>      296         {
>      297             if (aIter->second == aRef )
>      298                 aIter = m_aChildren.erase(aIter);
>      299             else
>      300                 ++aIter;
>      301         }
>      302     }
> See
> http://opengrok.libreoffice.org/xref/core/package/source/xstor/ohierarchyholder.cxx#298
>
> Is it ok to use "aEnd" or, since erase may  be called, we should change the
> while into:
> while (aIter != m_aChildren.end())
> (and remove aEnd)
> In this exact case (when the value may be present more times in the
> vector), you might want to use the Erase-remove idiom [1]:
>
> // remove all occurrences of aRef
> m_aChildren.erase(std::remove(m_aChildren.begin(), m_aChildren.end(), aRef), m_aChildren.end());
>
> Even with the comment I suppose, so that people who haven't read tons of
> C++ books can see what's going on ;-)
I copy pasted this exact line and it failed to compile :-(
> > Another thing: couldn't we break the loop after erase or could aRef be
> > present several times?
> Not sure - worth checking the history of that file I think.
According to git history, it's been like this since first git log, so 
let's let this.
About end iterator, it was my fault since I had created a const end 
iterator in the beginning of this year. I've fixed this (see 
http://cgit.freedesktop.org/libreoffice/core/commit/?id=9ebcd5ec54ec5d77cf46849f7f00bf915644f6e1)

Julien