Re: [PATCH] convert SV_DECL_PTRARR_DEL to boost::ptr_vector in sc/inc/viewuno.hxx

Hi Noel,

> From: Noel Grandin <noel@ubuntu.(none)>
Do you really want to be @ubuntu.(none)? If not, use
 $ git config --global user.email <your_address_here>

> -        for ( sal_uInt16 n=0; n<aActivationListeners.Count(); n++ )
> +        for (XActivationEventListenerVector::iterator it = aActivationListeners.begin(); it != 
> aActivationListeners.end(); ++it)
>          {
>              try
>              {
> -                (*aActivationListeners[n])->activeSpreadsheetChanged( aEvent );
> +                (*it)->activeSpreadsheetChanged( aEvent );
>              }
>              catch( uno::Exception& )
>              {
> -                aActivationListeners.DeleteAndDestroy( n );
> -                --n; // because it will be increased again in the loop
> +                it = aActivationListeners.erase( it);
>              }
>          }

'erase' returns an iterator to the *following* element, and that element 
will be skipped by "++it"; we must not increment the iterator after erase.
I don't know what is the most common idiom for this case, my proposal is

-        for ( sal_uInt16 n=0; n<aActivationListeners.Count(); n++ )
+        for (XActivationEventListenerVector::iterator it = 
aActivationListeners.begin(); it != aActivationListeners.end(); )
         {
             try
             {
-                (*aActivationListeners[n])->activeSpreadsheetChanged( 
aEvent );
+                (*it)->activeSpreadsheetChanged( aEvent );
+                ++it;
             }
             catch( uno::Exception& )
             {
-                aActivationListeners.DeleteAndDestroy( n );
-                --n; // because it will be increased again in the loop
+                it = aActivationListeners.erase( it);
             }
         }


Similarly, in the following example

> -    sal_uInt16 nCount = aMouseClickHandlers.Count();
> -    for ( sal_uInt16 n=nCount; n--; )
> +    sal_uInt16 nCount = aMouseClickHandlers.size();
> +    for (XMouseClickHandlerVector::iterator it = aMouseClickHandlers.begin(); it != 
> aMouseClickHandlers.end(); ++it)
>      {
> -        uno::Reference<awt::XEnhancedMouseClickHandler> *pObj = aMouseClickHandlers[n];
> +        uno::Reference<awt::XEnhancedMouseClickHandler> *pObj = &(*it);
>          if ( *pObj == aListener )
> -            aMouseClickHandlers.DeleteAndDestroy( n );
> +            it = aMouseClickHandlers.erase(it);
>      }

should be

-    sal_uInt16 nCount = aMouseClickHandlers.Count();
-    for ( sal_uInt16 n=nCount; n--; )
+    sal_uInt16 nCount = aMouseClickHandlers.size();
+    for (XMouseClickHandlerVector::iterator it = 
aMouseClickHandlers.begin(); it != aMouseClickHandlers.end();)
     {
-        uno::Reference<awt::XEnhancedMouseClickHandler> *pObj = 
aMouseClickHandlers[n];
+        uno::Reference<awt::XEnhancedMouseClickHandler> *pObj = &(*it);
         if ( *pObj == aListener )
-            aMouseClickHandlers.DeleteAndDestroy( n );
+            it = aMouseClickHandlers.erase(it);
+        else
+            ++it;
     }

or maybe it would be better without pObj:

-    sal_uInt16 nCount = aMouseClickHandlers.Count();
-    for ( sal_uInt16 n=nCount; n--; )
+    sal_uInt16 nCount = aMouseClickHandlers.size();
+    for (XMouseClickHandlerVector::iterator it = 
aMouseClickHandlers.begin(); it != aMouseClickHandlers.end();)
     {
-        uno::Reference<awt::XEnhancedMouseClickHandler> *pObj = 
aMouseClickHandlers[n];
-        if ( *pObj == aListener )
-            aMouseClickHandlers.DeleteAndDestroy( n );
+        if ( *it == aListener )
+            it = aMouseClickHandlers.erase(it);
+        else
+            ++it;
     }

Also, I *think* changing the loop order (descending->ascending) is safe 
here, so

> -    sal_uInt16 nCount = aPropertyChgListeners.Count();
> -    for ( sal_uInt16 n=nCount; n--; )
> +    for (size_t i = aPropertyChgListeners.size(); i--; )

may be converted to a iterator-based loop as well. Or is that intended?

Regards,
Ivan