[Libreoffice] [PATCH] Replaced tools/list with std::vector in funcdesc.hxx / funcdesc.cxx

Hi

In the attached patches (0001 is the real patch, 0002 contains three
comments missing in 0001) we have replaced use of tools/list by use of
std::vector in ScFunctionMgr and ScFunctionCategory (in
sc/inc/funcdesc.hxx and sc/source/core/funcdesc.cxx). We choose
vector, as there is done random access to the lists, while they are
only modified once (at creation). We changed the behaviour of
ScFunctionMgr slightly, as Get() now no longer resets the iterators
for First() and Next(), as this is quite counter intuitive. The patch
builds fine, and the behaviour of scalc seems unchanged.

The patch is joint work with Thies Pierdola, who is marked as author
in the patch file.

Regards
Sören Möller
(LGPLv3+/MPL for this and future patches)

From 4c70950d1338a760fa06705896e85506e7d4b574 Mon Sep 17 00:00:00 2001
From: Thies Pierdola <thiespierdola@gmail.com>
Date: Fri, 28 Jan 2011 19:39:07 +0100
Subject: [PATCH 1/2] Replaced tools/list with std::vector in ScFunctionMgr and ScFunctionCategory

---
 sc/inc/funcdesc.hxx              |   21 ++++++--
 sc/source/core/data/funcdesc.cxx |  107 ++++++++++++++++++++++---------------
 2 files changed, 81 insertions(+), 47 deletions(-)

diff --git a/sc/inc/funcdesc.hxx b/sc/inc/funcdesc.hxx
index 562f84c..12b60cf 100644
--- a/sc/inc/funcdesc.hxx
+++ b/sc/inc/funcdesc.hxx
@@ -185,6 +185,18 @@ public:
     */
     virtual bool isParameterOptional(sal_uInt32 _nPos) const ;
 
+    /**
+      Compares functions by name, respecting special characters
+
+      @param a
+      pointer to first function descriptor
+
+      @param b
+      pointer to second function descriptor
+
+      @return "(a < b)"
+    */
+    static bool compareByName(const ScFuncDesc* a, const ScFuncDesc* b);
 
     /**
       Stores whether a parameter is optional or suppressed
@@ -260,7 +272,7 @@ private:
 class ScFunctionCategory : public formula::IFunctionCategory
 {
 public:
-    ScFunctionCategory(ScFunctionMgr* _pMgr,List* _pCategory,sal_uInt32 _nCategory)
+    ScFunctionCategory(ScFunctionMgr* _pMgr,::std::vector<const ScFuncDesc*>* 
_pCategory,sal_uInt32 _nCategory)
             : m_pMgr(_pMgr),m_pCategory(_pCategory),m_nCategory(_nCategory){}
     virtual ~ScFunctionCategory(){}
 
@@ -288,7 +300,7 @@ public:
 
 private:
     ScFunctionMgr* m_pMgr; /**< function manager for this category */
-    List* m_pCategory; /**< list of functions in this category */
+    ::std::vector<const ScFuncDesc*>* m_pCategory; /**< list of functions in this category */
     mutable ::rtl::OUString m_sName; /**< name of this category */
     sal_uInt32 m_nCategory; /**< index number of this category */
 };
@@ -407,8 +419,9 @@ public:
 
 private:
     ScFunctionList* pFuncList; /**< list of all calc functions */
-    List*           aCatLists[MAX_FUNCCAT]; /**< array of all categories, 0 is the cumulative 
('All') category */
-    mutable List*   pCurCatList; /**< pointer to current category */
+    ::std::vector<const ScFuncDesc*>* aCatLists[MAX_FUNCCAT];
+    mutable ::std::vector<const ScFuncDesc*>::iterator pCurCatListIter;
+    mutable ::std::vector<const ScFuncDesc*>::iterator pCurCatListEnd;
 };
 
 #endif // SC_FUNCDESC_HXX
diff --git a/sc/source/core/data/funcdesc.cxx b/sc/source/core/data/funcdesc.cxx
index e58cbe4..892d643 100644
--- a/sc/source/core/data/funcdesc.cxx
+++ b/sc/source/core/data/funcdesc.cxx
@@ -363,6 +363,11 @@ bool ScFuncDesc::isParameterOptional(sal_uInt32 _nPos) const
     return pDefArgFlags[_nPos].bOptional;
 }
 
+bool ScFuncDesc::compareByName(const ScFuncDesc* a, const ScFuncDesc* b)
+{
+    return (ScGlobal::GetCaseCollator()->compareString(*a->pFuncName, *b->pFuncName ) == 
COMPARE_LESS);
+}
+
 //===================================================================
 // class ScFunctionList:
 //===================================================================
@@ -555,9 +560,13 @@ ScFunctionList::~ScFunctionList()
     }
 }
 
+//===================================================================
+// class ScFunctionCategory:
+//===================================================================
+
 sal_uInt32 ScFunctionCategory::getCount() const
 {
-    return m_pCategory->Count();
+    return m_pCategory->size();
 }
 
 const formula::IFunctionManager* ScFunctionCategory::getFunctionManager() const
@@ -575,9 +584,8 @@ const formula::IFunctionManager* ScFunctionCategory::getFunctionManager() const
 const formula::IFunctionDescription* ScFunctionCategory::getFunction(sal_uInt32 _nPos) const
 {
     const ScFuncDesc* pDesc = NULL;
-    sal_uInt32 i = 0;
-    for (pDesc = (const ScFuncDesc*)m_pCategory->First(); i < _nPos &&  pDesc; pDesc = (const 
ScFuncDesc*)m_pCategory->Next(),++i)
-        ;
+    if(_nPos < m_pCategory->size())
+        pDesc = m_pCategory->at(_nPos);
     return pDesc;
 }
 
@@ -591,42 +599,44 @@ sal_uInt32 ScFunctionCategory::getNumber() const
 //========================================================================
 
 ScFunctionMgr::ScFunctionMgr() :
-    pFuncList( ScGlobal::GetStarCalcFunctionList() ),
-    pCurCatList( NULL )
+    pFuncList( ScGlobal::GetStarCalcFunctionList() )
 {
     DBG_ASSERT( pFuncList, "Functionlist not found." );
-    sal_uInt32 nCount = pFuncList->GetCount();
-    ScFuncDesc* pDesc;
-    List* pRootList;
-    sal_uInt32 n;
+    sal_uInt32 catCount[MAX_FUNCCAT] = {0};
 
-    for (sal_uInt16 i = 0; i < MAX_FUNCCAT; ++i) // create category lists
-        aCatLists[i] = new List;
+    aCatLists[0] = new ::std::vector<const ScFuncDesc*>();
+    aCatLists[0]->reserve(pFuncList->GetCount());
 
-    pRootList = aCatLists[0]; // create cumulative list ("All")
-    CollatorWrapper* pCaseCollator = ScGlobal::GetCaseCollator();
-    for ( n=0; n<nCount; n++ )
+    // Retrieve all functions, store in cumulative ("All") category, and count
+    // number of functions in each category
+    for(const ScFuncDesc* pDesc = pFuncList->First(); pDesc; pDesc = pFuncList->Next())
     {
-        sal_uInt32 nTmpCnt=0;
-        pDesc = pFuncList->GetFunction(n);
-        for (nTmpCnt = 0; nTmpCnt < n; nTmpCnt++)
-        {
-            // it's case sensitiv, but special characters have to be put the right place
-            const ScFuncDesc* pTmpDesc = static_cast<const ScFuncDesc*>(
-                pRootList->GetObject(nTmpCnt));
-            if ( pCaseCollator->compareString(*pDesc->pFuncName, *pTmpDesc->pFuncName ) == 
COMPARE_LESS )
-                break;
-        }
-        pRootList->Insert(static_cast<void*>(pDesc), nTmpCnt); // insert the right place
+        DBG_ASSERT((pDesc->nCategory) < MAX_FUNCCAT, "Unknown category");
+        if ((pDesc->nCategory) < MAX_FUNCCAT)
+            ++catCount[pDesc->nCategory];
+        aCatLists[0]->push_back(pDesc);
     }
 
-    for ( n=0; n<nCount; n++ ) // copy to group list
+    // Sort functions in cumulative category by name
+    sort(aCatLists[0]->begin(), aCatLists[0]->end(), ScFuncDesc::compareByName);
+
+    // Allocate correct amount of space for categories
+    for (sal_uInt16 i = 1; i < MAX_FUNCCAT; ++i)
     {
-        pDesc = static_cast<ScFuncDesc*>(pRootList->GetObject(n));
-        DBG_ASSERT((pDesc->nCategory) < MAX_FUNCCAT, "Unknown category");
-        if ((pDesc->nCategory) < MAX_FUNCCAT)
-            aCatLists[pDesc->nCategory]->Insert(static_cast<void*>(pDesc), LIST_APPEND);
+        aCatLists[i] = new ::std::vector<const ScFuncDesc*>();
+        aCatLists[i]->reserve(catCount[i]);
     }
+
+    // Fill categories with the corresponding functions (still sorted by name)
+    for(::std::vector<const ScFuncDesc*>::iterator iter = aCatLists[0]->begin(); 
iter!=aCatLists[0]->end(); ++iter)
+    {
+        if (((*iter)->nCategory) < MAX_FUNCCAT)
+            aCatLists[(*iter)->nCategory]->push_back(*iter);
+    }
+
+    // Initialize iterators
+    pCurCatListIter = aCatLists[0]->end();
+    pCurCatListEnd = aCatLists[0]->end();
 }
 
 ScFunctionMgr::~ScFunctionMgr()
@@ -639,9 +649,13 @@ const ScFuncDesc* ScFunctionMgr::Get( const ::rtl::OUString& rFName ) const
 {
     const ScFuncDesc* pDesc = NULL;
     if (rFName.getLength() <= pFuncList->GetMaxFuncNameLen())
-        for (pDesc = First(0); pDesc; pDesc = Next())
-            if (rFName.equalsIgnoreAsciiCase(*pDesc->pFuncName))
-                break;
+    {
+        ScFuncDesc* dummy = new ScFuncDesc();
+        dummy->pFuncName = new ::rtl::OUString(rFName);
+        ::std::vector<const ScFuncDesc*>::iterator lower = lower_bound(aCatLists[0]->begin(), 
aCatLists[0]->end(), static_cast<const ScFuncDesc*>(dummy), ScFuncDesc::compareByName);
+        if(rFName.equalsIgnoreAsciiCase(*(*lower)->pFuncName))
+            pDesc = *lower;
+    }
     return pDesc;
 }
 
@@ -657,25 +671,32 @@ const ScFuncDesc* ScFunctionMgr::Get( sal_uInt16 nFIndex ) const
 const ScFuncDesc* ScFunctionMgr::First( sal_uInt16 nCategory ) const
 {
     DBG_ASSERT( nCategory < MAX_FUNCCAT, "Unbekannte Kategorie" );
-
+    const ScFuncDesc* pDesc = NULL;
     if ( nCategory < MAX_FUNCCAT )
     {
-        pCurCatList = aCatLists[nCategory];
-        return (const ScFuncDesc*)pCurCatList->First();
+        pCurCatListIter = aCatLists[nCategory]->begin();
+        pCurCatListEnd = aCatLists[nCategory]->end();
+        pDesc = *pCurCatListIter;
     }
     else
     {
-        pCurCatList = NULL;
-        return NULL;
+        pCurCatListIter = aCatLists[0]->end();
+        pCurCatListEnd = aCatLists[0]->end();
     }
+    return pDesc;
 }
 
 const ScFuncDesc* ScFunctionMgr::Next() const
 {
-    if ( pCurCatList )
-        return (const ScFuncDesc*)pCurCatList->Next();
-    else
-        return NULL;
+    const ScFuncDesc* pDesc = NULL;
+    if ( pCurCatListIter != pCurCatListEnd )
+    {
+        if ( (++pCurCatListIter) != pCurCatListEnd )
+        {
+            pDesc = *pCurCatListIter;
+        }
+    }
+    return pDesc;
 }
 
 sal_uInt32 ScFunctionMgr::getCount() const
-- 
1.7.0.4

From 9490f6186ae972c5be79a11b4e57228342a45ec4 Mon Sep 17 00:00:00 2001
From: Thies Pierdola <thiespierdola@gmail.com>
Date: Fri, 28 Jan 2011 19:44:02 +0100
Subject: [PATCH 2/2] Comments for private variables in ScFunctionMgr

---
 sc/inc/funcdesc.hxx |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sc/inc/funcdesc.hxx b/sc/inc/funcdesc.hxx
index 12b60cf..49909c5 100644
--- a/sc/inc/funcdesc.hxx
+++ b/sc/inc/funcdesc.hxx
@@ -419,9 +419,9 @@ public:
 
 private:
     ScFunctionList* pFuncList; /**< list of all calc functions */
-    ::std::vector<const ScFuncDesc*>* aCatLists[MAX_FUNCCAT];
-    mutable ::std::vector<const ScFuncDesc*>::iterator pCurCatListIter;
-    mutable ::std::vector<const ScFuncDesc*>::iterator pCurCatListEnd;
+    ::std::vector<const ScFuncDesc*>* aCatLists[MAX_FUNCCAT]; /**< array of all categories, 0 is 
the cumulative ('All') category */
+    mutable ::std::vector<const ScFuncDesc*>::iterator pCurCatListIter; /**< position in current 
category */
+    mutable ::std::vector<const ScFuncDesc*>::iterator pCurCatListEnd; /**< end of current 
category */
 };
 
 #endif // SC_FUNCDESC_HXX
-- 
1.7.0.4