Re: [WIP] Remove boost dependency in sc/source/filter/excel/excimp8.cxx

Albert,

Thanks for the patch!

> It would be great if I could get some advice on how to move forward on
> removing more of the boost dependency.

I'm definitely not the most experienced person here, but I can try to
give you some pointers.

>     for( const auto& rFilterPtr : maFilters )
>         rFilterPtr->Apply();

Awesome! Much more readable.

> Should i modify it to return XclImpAutoFilterSharePtr instead?

I personally wouldn't. I would use std::shared_ptr::get()
(http://en.cppreference.com/w/cpp/memory/shared_ptr/get)

One more random comment. In general std::make_shared
(http://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared)
is better than constructing using std::shared_ptr<T>::shared_ptr(T*)
and then copying. That will result in two heap allocs vs. make_shared
which should just perform one. The following stack-overflow question
sums up the pros and cons of make_shared quite nicely.
(http://stackoverflow.com/questions/20895648/difference-in-make-shared-and-normal-shared-ptr-in-c)

Again, I still have a lot to learn, so take everything I say with a
grain of salt, but I think you're definitely on the right track. I
attached a patch combining what you had and the above random points.

Best,

Daniel

On Sun, Sep 13, 2015 at 10:06:05AM +0200, Albert Thuswaldner wrote:
> Hi,
> It would be great if I could get some advice on how to move forward on
> removing more of the boost dependency.
>
> Originally I was working on this:
> [bug 93243] replace boost::bind with C++11 lambdas
>
> Line 18: sc/source/filter/excel/excimp8.cxx
>     std::for_each(maFilters.begin(),maFilters.end(),
>         boost::bind(&XclImpAutoFilterData::Apply,_1));
>
> Searching the code I hav found examples where this type of for_each
> loop is replaced by a simple for loop:
>
>     for( const auto& rFilterPtr : maFilters )
>         rFilterPtr->Apply();
>
> This requires some additional modificatioins to the code which can be
> seen in the attached patch.
>
> The question which is left is what to do with what the function below
> should return:
>
> XclImpAutoFilterData* XclImpAutoFilterBuffer::GetByTab( SCTAB nTab )
>
> Should i modify it to return XclImpAutoFilterSharePtr instead?
> Am I on the right track in general?
>
> Thanks for the help!
>
> /Albert

> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice

From 5a167b44343e70fce8b194bcb9ee35ebc41b9a2c Mon Sep 17 00:00:00 2001
From: Daniel Robertson <danlrobertson89@gmail.com>
Date: Sun, 13 Sep 2015 17:29:28 -0400
Subject: [PATCH] tdf#93243 replace boost::bind with C++11 lambdas

Replace complex usages of boost::bind and std::for_each with range-based
for-loops and lambdas when appropriate.

Change-Id: I7299cb22ed844af0256a46ce93d9aae8e9104195
---
 sc/source/filter/excel/excimp8.cxx | 16 +++++++---------
 sc/source/filter/inc/excimp8.hxx   |  6 ++----
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/sc/source/filter/excel/excimp8.cxx b/sc/source/filter/excel/excimp8.cxx
index eba2d5c..3a47f25 100644
--- a/sc/source/filter/excel/excimp8.cxx
+++ b/sc/source/filter/excel/excimp8.cxx
@@ -21,8 +21,6 @@
 
 #include "excimp8.hxx"
 
-#include <boost/bind.hpp>
-
 #include <scitems.hxx>
 #include <comphelper/processfactory.hxx>
 #include <unotools/fltrcfg.hxx>
@@ -846,7 +844,8 @@ void XclImpAutoFilterData::EnableRemoveFilter()
 void XclImpAutoFilterBuffer::Insert( RootData* pRoot, const ScRange& rRange)
 {
     if( !GetByTab( rRange.aStart.Tab() ) )
-        maFilters.push_back( new XclImpAutoFilterData( pRoot, rRange) );
+        maFilters.push_back( std::make_shared< XclImpAutoFilterData >(
+                XclImpAutoFilterData( pRoot, rRange) ) );
 }
 
 void XclImpAutoFilterBuffer::AddAdvancedRange( const ScRange& rRange )
@@ -865,17 +864,16 @@ void XclImpAutoFilterBuffer::AddExtractPos( const ScRange& rRange )
 
 void XclImpAutoFilterBuffer::Apply()
 {
-    std::for_each(maFilters.begin(),maFilters.end(),
-        boost::bind(&XclImpAutoFilterData::Apply,_1));
+    for( const auto& rFilterPtr : maFilters )
+        rFilterPtr->Apply();
 }
 
 XclImpAutoFilterData* XclImpAutoFilterBuffer::GetByTab( SCTAB nTab )
 {
-    boost::ptr_vector<XclImpAutoFilterData>::iterator it;
-    for( it = maFilters.begin(); it != maFilters.end(); ++it )
+    for( const auto& rFilterPtr : maFilters )
     {
-        if( it->Tab() == nTab )
-            return &(*it);
+        if( rFilterPtr->Tab() == nTab )
+            return rFilterPtr.get();
     }
     return NULL;
 }
diff --git a/sc/source/filter/inc/excimp8.hxx b/sc/source/filter/inc/excimp8.hxx
index e50cfd4..2ce1135 100644
--- a/sc/source/filter/inc/excimp8.hxx
+++ b/sc/source/filter/inc/excimp8.hxx
@@ -22,8 +22,6 @@
 
 #include <string.h>
 
-#include <boost/ptr_container/ptr_vector.hpp>
-
 #include "imp_op.hxx"
 #include "root.hxx"
 #include "excscen.hxx"
@@ -123,8 +121,8 @@ public:
     XclImpAutoFilterData*       GetByTab( SCTAB nTab );
 
 private:
-
-    boost::ptr_vector<XclImpAutoFilterData> maFilters;
+    typedef std::shared_ptr<XclImpAutoFilterData> XclImpAutoFilterSharePtr;
+    std::vector<XclImpAutoFilterSharePtr> maFilters;
 };
 
 #endif
-- 
2.5.1

Attachment: signature.asc
Description: Digital signature