sc/inc/dpobject.hxx | 2 sc/qa/unit/ucalc.hxx | 6 ++ sc/qa/unit/ucalc_pivottable.cxx | 88 +++++++++++++++++++++++++++++++++++++++ sc/source/core/data/dpobject.cxx | 30 ++++++------- 4 files changed, 109 insertions(+), 17 deletions(-)
New commits: commit d4c09a4e3737c1355c666299d8a3f3f30dc659dd Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk> Date: Mon Feb 6 20:35:51 2017 +0100 sc: simplify GetByName, FreeTable methods of DPCollection + test Simplify DPCollection GetByName and FreeTable by using c++11 features. Change GetByName to return non-const ScDPObject as this is more useful (as operator[] does that already anyway). Change-Id: Ia502c615acc5cb7fdc51acea9b428d04e1c9a40f Reviewed-on: https://gerrit.libreoffice.org/34002 Tested-by: Jenkins <c...@libreoffice.org> Reviewed-by: Tomaž Vajngerl <qui...@gmail.com> diff --git a/sc/inc/dpobject.hxx b/sc/inc/dpobject.hxx index 2f144c5..d8c20f1 100644 --- a/sc/inc/dpobject.hxx +++ b/sc/inc/dpobject.hxx @@ -371,7 +371,7 @@ public: SC_DLLPUBLIC ScDPObject& operator[](size_t nIndex); SC_DLLPUBLIC const ScDPObject& operator[](size_t nIndex) const; - const ScDPObject* GetByName(const OUString& rName) const; + ScDPObject* GetByName(const OUString& rName) const; void DeleteOnTab( SCTAB nTab ); void UpdateReference( UpdateRefMode eUpdateRefMode, diff --git a/sc/qa/unit/ucalc.hxx b/sc/qa/unit/ucalc.hxx index 1715447..f324748 100644 --- a/sc/qa/unit/ucalc.hxx +++ b/sc/qa/unit/ucalc.hxx @@ -320,6 +320,11 @@ public: */ void testPivotTableRepeatItemLabels(); + /** + * Test DPCollection public methods + */ + void testPivotTableDPCollection(); + void testCellCopy(); void testSheetCopy(); void testSheetMove(); @@ -621,6 +626,7 @@ public: CPPUNIT_TEST(testPivotTableFieldReference); CPPUNIT_TEST(testPivotTableDocFunc); CPPUNIT_TEST(testPivotTableRepeatItemLabels); + CPPUNIT_TEST(testPivotTableDPCollection); CPPUNIT_TEST(testCellCopy); CPPUNIT_TEST(testSheetCopy); CPPUNIT_TEST(testSheetMove); diff --git a/sc/qa/unit/ucalc_pivottable.cxx b/sc/qa/unit/ucalc_pivottable.cxx index a7c5037..28b475d 100644 --- a/sc/qa/unit/ucalc_pivottable.cxx +++ b/sc/qa/unit/ucalc_pivottable.cxx @@ -2401,5 +2401,93 @@ void Test::testPivotTableRepeatItemLabels() m_pDoc->DeleteTab(0); } +void Test::testPivotTableDPCollection() +{ + m_pDoc->InsertTab(0, "Data"); + m_pDoc->InsertTab(1, "Table"); + + // Dimension definition + DPFieldDef aFields[] = { + { "Software", sheet::DataPilotFieldOrientation_ROW, 0, false }, + { "Version", sheet::DataPilotFieldOrientation_COLUMN, 0, false }, + { "1.2.3", sheet::DataPilotFieldOrientation_DATA, 0, false } + }; + + // Raw data + const char* aData[][3] = { + { "LibreOffice", "3.3.0", "30" }, + { "LibreOffice", "3.3.1", "20" }, + { "LibreOffice", "3.4.0", "45" }, + }; + + size_t nFieldCount = SAL_N_ELEMENTS(aFields); + size_t nDataCount = SAL_N_ELEMENTS(aData); + + ScRange aSrcRange = insertDPSourceData(m_pDoc, aFields, nFieldCount, aData, nDataCount); + SCROW nRow1 = aSrcRange.aStart.Row(), nRow2 = aSrcRange.aEnd.Row(); + SCCOL nCol1 = aSrcRange.aStart.Col(), nCol2 = aSrcRange.aEnd.Col(); + ScRange aDataRange(nCol1, nRow1, 0, nCol2, nRow2, 0); + + ScDPCollection* pDPs = m_pDoc->GetDPCollection(); + bool bSuccess = false; + + // Check at the beginning + CPPUNIT_ASSERT_EQUAL_MESSAGE("there should be no DP table", size_t(0), pDPs->GetCount()); + + CPPUNIT_ASSERT_EQUAL_MESSAGE("should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("DP1")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("")); + + // Add 2 DP objects + ScDPObject* pDPObj = createDPFromRange(m_pDoc, aDataRange , aFields, nFieldCount, false); + bSuccess = pDPs->InsertNewTable(pDPObj); + CPPUNIT_ASSERT_MESSAGE("failed to insert a new DP object into document", bSuccess); + pDPObj->SetName("DP1"); // set custom name + + CPPUNIT_ASSERT_EQUAL_MESSAGE("there should be only one data pilot table.", size_t(1), pDPs->GetCount()); + + ScDPObject* pDPObj2 = createDPFromRange(m_pDoc, aDataRange, aFields, nFieldCount, false); + bSuccess = pDPs->InsertNewTable(pDPObj2); + CPPUNIT_ASSERT_MESSAGE("failed to insert a new DP object into document", bSuccess); + pDPObj2->SetName("DP2"); // set custom name + + CPPUNIT_ASSERT_EQUAL_MESSAGE("there should be two DP tables", size_t(2), pDPs->GetCount()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("should return first DPObject", + pDPObj, pDPs->GetByName("DP1")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("should return second DPObject", + pDPObj2, pDPs->GetByName("DP2")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("empty string should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("non existent name should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("Non")); + // Remove first DP Object + pDPs->FreeTable(pDPObj); + CPPUNIT_ASSERT_EQUAL_MESSAGE("there should be only one DP table", size_t(1), pDPs->GetCount()); + + CPPUNIT_ASSERT_EQUAL_MESSAGE("first DP object was deleted, should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("DP1")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("should return second DPObject", + pDPObj2, pDPs->GetByName("DP2")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("empty string should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("non existent name should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("Non")); + + // Remove second DP Object + pDPs->FreeTable(pDPObj2); + CPPUNIT_ASSERT_EQUAL_MESSAGE("first DP object was deleted, should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("DP1")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("second DP object was deleted, should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("DP2")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("empty string should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("")); + CPPUNIT_ASSERT_EQUAL_MESSAGE("non existent name should return nullptr", + static_cast<ScDPObject*>(nullptr), pDPs->GetByName("Non")); + + // Clean-up + m_pDoc->DeleteTab(1); + m_pDoc->DeleteTab(0); +} /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/source/core/data/dpobject.cxx b/sc/source/core/data/dpobject.cxx index c413220..9b7c6e2 100644 --- a/sc/source/core/data/dpobject.cxx +++ b/sc/source/core/data/dpobject.cxx @@ -3734,12 +3734,13 @@ const ScDPObject& ScDPCollection::operator [](size_t nIndex) const return *maTables[nIndex].get(); } -const ScDPObject* ScDPCollection::GetByName(const OUString& rName) const +ScDPObject* ScDPCollection::GetByName(const OUString& rName) const { - TablesType::const_iterator itr = maTables.begin(), itrEnd = maTables.end(); - for (; itr != itrEnd; ++itr) - if ((*itr)->GetName() == rName) - return itr->get(); + for (std::unique_ptr<ScDPObject> const & pObject : maTables) + { + if (pObject->GetName() == rName) + return pObject.get(); + } return nullptr; } @@ -3771,22 +3772,19 @@ OUString ScDPCollection::CreateNewName() const return OUString(); // should not happen } -void ScDPCollection::FreeTable(ScDPObject* pDPObj) +void ScDPCollection::FreeTable(ScDPObject* pDPObject) { - const ScRange& rOutRange = pDPObj->GetOutRange(); + const ScRange& rOutRange = pDPObject->GetOutRange(); const ScAddress& s = rOutRange.aStart; const ScAddress& e = rOutRange.aEnd; mpDoc->RemoveFlagsTab(s.Col(), s.Row(), e.Col(), e.Row(), s.Tab(), ScMF::DpTable); - TablesType::iterator itr = maTables.begin(), itrEnd = maTables.end(); - for (; itr != itrEnd; ++itr) + + auto funcRemoveCondition = [pDPObject] (std::unique_ptr<ScDPObject> const & pCurrent) { - ScDPObject* p = itr->get(); - if (p == pDPObj) - { - maTables.erase(itr); - break; - } - } + return pCurrent.get() == pDPObject; + }; + + maTables.erase(std::remove_if(maTables.begin(), maTables.end(), funcRemoveCondition), maTables.end()); } bool ScDPCollection::InsertNewTable(ScDPObject* pDPObj)
_______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits