[Libreoffice-commits] core.git: sc/inc sc/qa sc/source
Kohei Yoshida
kohei.yoshida at collabora.com
Mon Jan 6 13:14:15 PST 2014
sc/inc/column.hxx | 7 -
sc/inc/table.hxx | 4 -
sc/qa/unit/ucalc.cxx | 139 +++++++++++++++++++++++++++++++++++++
sc/qa/unit/ucalc.hxx | 7 +
sc/source/core/data/column2.cxx | 146 ++++++++++++++-------------------------
sc/source/core/data/documen4.cxx | 17 +---
sc/source/core/data/table3.cxx | 31 ++------
7 files changed, 218 insertions(+), 133 deletions(-)
New commits:
commit 4a7a6b46c0dc779581f271b9e6c13c365eca7ab8
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date: Mon Jan 6 16:12:28 2014 -0500
fdo#73001: Simplify the selection function logic & calculate correct results.
Fixing a bug and cleaning up the code all at the same time. And don't forget
to write test for it as well.
Change-Id: Ia0322c4bebd4c5debcbfa4bb0902afbe581208b2
diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index 6ec3336..8b09db3 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -237,13 +237,8 @@ public:
ScAttrIterator* CreateAttrIterator( SCROW nStartRow, SCROW nEndRow ) const;
- // UpdateSelectionFunction: multi-select
void UpdateSelectionFunction(
- const ScMarkData& rMark, ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows,
- bool bDoExclude, SCROW nExStartRow, SCROW nExEndRow );
-
- void UpdateAreaFunction(
- ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows, SCROW nStartRow, SCROW nEndRow);
+ const ScMarkData& rMark, ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows );
void CopyToColumn(
sc::CopyToDocContext& rCxt, SCROW nRow1, SCROW nRow2, sal_uInt16 nFlags, bool bMarked,
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 09d541b..68a1791 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -542,9 +542,7 @@ public:
double nStepValue, double nMaxValue, ScProgress* pProgress);
OUString GetAutoFillPreview( const ScRange& rSource, SCCOL nEndX, SCROW nEndY );
- void UpdateSelectionFunction( ScFunctionData& rData,
- SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW nEndRow,
- const ScMarkData& rMark );
+ void UpdateSelectionFunction( ScFunctionData& rData, const ScMarkData& rMark );
void AutoFormat( SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW nEndRow,
sal_uInt16 nFormatNo );
diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index cd0a9f3..5196c98 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -676,6 +676,145 @@ void Test::testDataEntries()
m_pDoc->DeleteTab(0);
}
+void Test::testSelectionFunction()
+{
+ m_pDoc->InsertTab(0, "Test");
+
+ // Insert values into B2:B4.
+ m_pDoc->SetString(ScAddress(1,1,0), "=1"); // formula
+ m_pDoc->SetValue(ScAddress(1,2,0), 2.0);
+ m_pDoc->SetValue(ScAddress(1,3,0), 3.0);
+
+ // Insert strings into B5:B8.
+ m_pDoc->SetString(ScAddress(1,4,0), "A");
+ m_pDoc->SetString(ScAddress(1,5,0), "B");
+ m_pDoc->SetString(ScAddress(1,6,0), "=\"C\""); // formula
+ m_pDoc->SetString(ScAddress(1,7,0), "D");
+
+ // Insert values into D2:D4.
+ m_pDoc->SetValue(ScAddress(3,1,0), 4.0);
+ m_pDoc->SetValue(ScAddress(3,2,0), 5.0);
+ m_pDoc->SetValue(ScAddress(3,3,0), 6.0);
+
+ // Insert edit text into D5.
+ ScFieldEditEngine& rEE = m_pDoc->GetEditEngine();
+ rEE.SetText("Rich Text");
+ m_pDoc->SetEditText(ScAddress(3,4,0), rEE.CreateTextObject());
+
+ // Insert Another string into D6.
+ m_pDoc->SetString(ScAddress(3,5,0), "E");
+
+ // Select B2:B8 & D2:D8 disjoint region.
+ ScRangeList aRanges;
+ aRanges.Append(ScRange(1,1,0,1,7,0)); // B2:B8
+ aRanges.Append(ScRange(3,1,0,3,7,0)); // D2:D8
+ ScMarkData aMark;
+ aMark.MarkFromRangeList(aRanges, true);
+
+ struct Check
+ {
+ ScSubTotalFunc meFunc;
+ double mfExpected;
+ };
+
+ {
+ Check aChecks[] =
+ {
+ { SUBTOTAL_FUNC_AVE, 3.5 },
+ { SUBTOTAL_FUNC_CNT2, 12.0 },
+ { SUBTOTAL_FUNC_CNT, 6.0 },
+ { SUBTOTAL_FUNC_MAX, 6.0 },
+ { SUBTOTAL_FUNC_MIN, 1.0 },
+ { SUBTOTAL_FUNC_SUM, 21.0 },
+ { SUBTOTAL_FUNC_SELECTION_COUNT, 14.0 }
+ };
+
+ for (size_t i = 0; i < SAL_N_ELEMENTS(aChecks); ++i)
+ {
+ double fRes = 0.0;
+ bool bRes = m_pDoc->GetSelectionFunction(aChecks[i].meFunc, ScAddress(), aMark, fRes);
+ CPPUNIT_ASSERT_MESSAGE("Failed to fetch selection function result.", bRes);
+ CPPUNIT_ASSERT_EQUAL(aChecks[i].mfExpected, fRes);
+ }
+ }
+
+ // Hide rows 4 and 6 and check the results again.
+
+ m_pDoc->SetRowHidden(3, 3, 0, true);
+ m_pDoc->SetRowHidden(5, 5, 0, true);
+ CPPUNIT_ASSERT_MESSAGE("This row should be hidden.", m_pDoc->RowHidden(3, 0));
+ CPPUNIT_ASSERT_MESSAGE("This row should be hidden.", m_pDoc->RowHidden(5, 0));
+
+ {
+ Check aChecks[] =
+ {
+ { SUBTOTAL_FUNC_AVE, 3.0 },
+ { SUBTOTAL_FUNC_CNT2, 8.0 },
+ { SUBTOTAL_FUNC_CNT, 4.0 },
+ { SUBTOTAL_FUNC_MAX, 5.0 },
+ { SUBTOTAL_FUNC_MIN, 1.0 },
+ { SUBTOTAL_FUNC_SUM, 12.0 },
+ { SUBTOTAL_FUNC_SELECTION_COUNT, 10.0 }
+ };
+
+ for (size_t i = 0; i < SAL_N_ELEMENTS(aChecks); ++i)
+ {
+ double fRes = 0.0;
+ bool bRes = m_pDoc->GetSelectionFunction(aChecks[i].meFunc, ScAddress(), aMark, fRes);
+ CPPUNIT_ASSERT_MESSAGE("Failed to fetch selection function result.", bRes);
+ CPPUNIT_ASSERT_EQUAL(aChecks[i].mfExpected, fRes);
+ }
+ }
+
+ // Make sure that when no selection is present, use the current cursor position.
+ ScMarkData aEmpty;
+
+ {
+ // D3 (numeric cell containing 5.)
+ ScAddress aPos(3, 2, 0);
+
+ Check aChecks[] =
+ {
+ { SUBTOTAL_FUNC_AVE, 5.0 },
+ { SUBTOTAL_FUNC_CNT2, 1.0 },
+ { SUBTOTAL_FUNC_CNT, 1.0 },
+ { SUBTOTAL_FUNC_MAX, 5.0 },
+ { SUBTOTAL_FUNC_MIN, 5.0 },
+ { SUBTOTAL_FUNC_SUM, 5.0 },
+ { SUBTOTAL_FUNC_SELECTION_COUNT, 1.0 }
+ };
+
+ for (size_t i = 0; i < SAL_N_ELEMENTS(aChecks); ++i)
+ {
+ double fRes = 0.0;
+ bool bRes = m_pDoc->GetSelectionFunction(aChecks[i].meFunc, aPos, aEmpty, fRes);
+ CPPUNIT_ASSERT_MESSAGE("Failed to fetch selection function result.", bRes);
+ CPPUNIT_ASSERT_EQUAL(aChecks[i].mfExpected, fRes);
+ }
+ }
+
+ {
+ // B7 (string formula cell containing ="C".)
+ ScAddress aPos(1, 6, 0);
+
+ Check aChecks[] =
+ {
+ { SUBTOTAL_FUNC_CNT2, 1.0 },
+ { SUBTOTAL_FUNC_SELECTION_COUNT, 1.0 }
+ };
+
+ for (size_t i = 0; i < SAL_N_ELEMENTS(aChecks); ++i)
+ {
+ double fRes = 0.0;
+ bool bRes = m_pDoc->GetSelectionFunction(aChecks[i].meFunc, aPos, aEmpty, fRes);
+ CPPUNIT_ASSERT_MESSAGE("Failed to fetch selection function result.", bRes);
+ CPPUNIT_ASSERT_EQUAL(aChecks[i].mfExpected, fRes);
+ }
+ }
+
+ m_pDoc->DeleteTab(0);
+}
+
void Test::testCopyAttributes()
{
CPPUNIT_ASSERT_MESSAGE ("mashed up attributes", !(IDF_ATTRIB & IDF_CONTENTS));
diff --git a/sc/qa/unit/ucalc.hxx b/sc/qa/unit/ucalc.hxx
index 6bbc86a..98528f6 100644
--- a/sc/qa/unit/ucalc.hxx
+++ b/sc/qa/unit/ucalc.hxx
@@ -90,6 +90,12 @@ public:
*/
void testDataEntries();
+ /**
+ * Selection function is responsible for displaying quick calculation
+ * results in the status bar.
+ */
+ void testSelectionFunction();
+
void testFormulaCreateStringFromTokens();
void testFormulaParseReference();
void testFetchVectorRefArray();
@@ -306,6 +312,7 @@ public:
CPPUNIT_TEST(testRangeList);
CPPUNIT_TEST(testInput);
CPPUNIT_TEST(testDataEntries);
+ CPPUNIT_TEST(testSelectionFunction);
CPPUNIT_TEST(testFormulaCreateStringFromTokens);
CPPUNIT_TEST(testFormulaParseReference);
CPPUNIT_TEST(testFetchVectorRefArray);
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 60b7b42..4d7b4bf 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -3254,7 +3254,6 @@ namespace {
class UpdateSubTotalHandler
{
ScFunctionData& mrData;
- ScFlatBoolRowSegments& mrHiddenRows;
void update(double fVal, bool bVal)
{
@@ -3311,22 +3310,25 @@ class UpdateSubTotalHandler
}
public:
- UpdateSubTotalHandler(ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows) :
- mrData(rData), mrHiddenRows(rHiddenRows) {}
+ UpdateSubTotalHandler(ScFunctionData& rData) : mrData(rData) {}
- void operator() (size_t nRow, double fVal)
+ void operator() (size_t /*nRow*/, double fVal)
{
- if (mrHiddenRows.getValue(nRow))
- return;
-
update(fVal, true);
}
- void operator() (size_t nRow, ScFormulaCell* pCell)
+ void operator() (size_t /*nRow*/, const svl::SharedString&)
{
- if (mrHiddenRows.getValue(nRow))
- return;
+ update(0.0, false);
+ }
+ void operator() (size_t /*nRow*/, const EditTextObject*)
+ {
+ update(0.0, false);
+ }
+
+ void operator() (size_t /*nRow*/, ScFormulaCell* pCell)
+ {
double fVal = 0.0;
bool bVal = false;
if (mrData.eFunc != SUBTOTAL_FUNC_CNT2) // it doesn't interest us
@@ -3353,99 +3355,63 @@ public:
// multiple selections:
void ScColumn::UpdateSelectionFunction(
- const ScMarkData& rMark, ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows,
- bool bDoExclude, SCROW nExStartRow, SCROW nExEndRow)
+ const ScMarkData& rMark, ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows )
{
- if ( rData.eFunc != SUBTOTAL_FUNC_SELECTION_COUNT )
+ sc::SingleColumnSpanSet aSpanSet;
+ aSpanSet.scan(rMark, nTab, nCol); // mark all selected rows.
+
+ // Exclude all hidden rows.
+ ScFlatBoolRowSegments::RangeData aRange;
+ SCROW nRow = 0;
+ while (nRow <= MAXROW)
{
- sc::SingleColumnSpanSet aSpanSet;
- aSpanSet.scan(rMark, nTab, nCol);
- if (bDoExclude)
- {
- aSpanSet.set(0, nExStartRow, false);
- aSpanSet.set(nExEndRow+1, MAXROWCOUNT, false);
- }
+ if (!rHiddenRows.getRangeData(nRow, aRange))
+ break;
- sc::SingleColumnSpanSet::SpansType aSpans;
- aSpanSet.getSpans(aSpans);
- UpdateSubTotalHandler aFunc(rData, rHiddenRows);
- sc::CellStoreType::iterator itCellPos = maCells.begin();
- sc::SingleColumnSpanSet::SpansType::const_iterator it = aSpans.begin(), itEnd = aSpans.end();
- for (; it != itEnd; ++it)
- {
- itCellPos = sc::ProcessFormulaNumeric(
- itCellPos, maCells, it->mnRow1, it->mnRow2, aFunc);
- }
+ if (aRange.mbValue)
+ // Hidden range detected.
+ aSpanSet.set(nRow, aRange.mnRow2, false);
+
+ nRow = aRange.mnRow2 + 1;
}
- else
- {
- SCROW nTop, nBottom;
- // ScMarkData::GetArray() returns a valid array only if
- // 'rMark.IsMultiMarked()' returns true.
- // Since ScTable::UpdateSelectionFunction() already checked that first
- // before calling this method it does not need to be repeated here.
+ sc::SingleColumnSpanSet::SpansType aSpans;
+ aSpanSet.getSpans(aSpans);
- ScMarkArrayIter aIter(rMark.GetArray() + nCol);
- ScFlatBoolRowSegments::RangeData aData;
+ sc::SingleColumnSpanSet::SpansType::const_iterator it = aSpans.begin(), itEnd = aSpans.end();
- while (aIter.Next( nTop, nBottom ))
+ switch (rData.eFunc)
+ {
+ case SUBTOTAL_FUNC_SELECTION_COUNT:
{
- sal_Int32 nCellCount = 0; // to get the count of selected visible cells
- SCROW nRow = nTop;
-
- while ( nRow <= nBottom )
+ // Simply count selected rows regardless of cell contents.
+ for (; it != itEnd; ++it)
+ rData.nCount += it->mnRow2 - it->mnRow1 + 1;
+ }
+ break;
+ case SUBTOTAL_FUNC_CNT2:
+ {
+ // We need to parse all non-empty cells.
+ sc::CellStoreType::const_iterator itCellPos = maCells.begin();
+ UpdateSubTotalHandler aFunc(rData);
+ for (; it != itEnd; ++it)
{
- if (!rHiddenRows.getRangeData(nRow, aData)) // failed to get range data
- break;
-
- if (aData.mnRow2 > nBottom)
- aData.mnRow2 = nBottom;
-
- if (!aData.mbValue)
- {
- nCellCount += aData.mnRow2 - nRow + 1;
-
- // Till this point, nCellCount also includes count of those cells which are excluded
- // So, they should be decremented now.
-
- if ( bDoExclude && nExStartRow >= nRow && nExEndRow <= aData.mnRow2 )
- nCellCount -= nExEndRow - nExStartRow + 1;
- }
- nRow = aData.mnRow2 + 1;
+ itCellPos = sc::ParseAllNonEmpty(
+ itCellPos, maCells, it->mnRow1, it->mnRow2, aFunc);
}
- rData.nCount += nCellCount;
}
- }
-}
-
-// with bNoMarked ignore the multiple selections
-void ScColumn::UpdateAreaFunction(
- ScFunctionData& rData, ScFlatBoolRowSegments& rHiddenRows, SCROW nStartRow, SCROW nEndRow)
-{
- if ( rData.eFunc != SUBTOTAL_FUNC_SELECTION_COUNT )
- {
- UpdateSubTotalHandler aFunc(rData, rHiddenRows);
- sc::ProcessFormulaNumeric(
- maCells.begin(), maCells, nStartRow, nEndRow, aFunc);
- }
- else
- {
- sal_Int32 nCellCount = 0; // to get the count of selected visible cells
- SCROW nRow = nStartRow;
- ScFlatBoolRowSegments::RangeData aData;
-
- while (nRow <= nEndRow)
+ break;
+ default:
{
- if (!rHiddenRows.getRangeData(nRow, aData))
- break;
-
- if (!aData.mbValue)
- nCellCount += (aData.mnRow2 <= nEndRow ? aData.mnRow2 : nEndRow) - nRow + 1;
-
- nRow = aData.mnRow2 + 1;
+ // We need to parse only numeric values.
+ sc::CellStoreType::const_iterator itCellPos = maCells.begin();
+ UpdateSubTotalHandler aFunc(rData);
+ for (; it != itEnd; ++it)
+ {
+ itCellPos = sc::ParseFormulaNumeric(
+ itCellPos, maCells, it->mnRow1, it->mnRow2, aFunc);
+ }
}
- rData.nCount += nCellCount;
}
}
diff --git a/sc/source/core/data/documen4.cxx b/sc/source/core/data/documen4.cxx
index 0aa6383..ed49696 100644
--- a/sc/source/core/data/documen4.cxx
+++ b/sc/source/core/data/documen4.cxx
@@ -613,22 +613,17 @@ bool ScDocument::GetSelectionFunction( ScSubTotalFunc eFunc,
{
ScFunctionData aData(eFunc);
- ScRange aSingle( rCursor );
- if ( rMark.IsMarked() )
- rMark.GetMarkArea(aSingle);
-
- SCCOL nStartCol = aSingle.aStart.Col();
- SCROW nStartRow = aSingle.aStart.Row();
- SCCOL nEndCol = aSingle.aEnd.Col();
- SCROW nEndRow = aSingle.aEnd.Row();
+ ScMarkData aMark(rMark);
+ aMark.MarkToMulti();
+ if (!aMark.IsMultiMarked())
+ aMark.SetMarkArea(rCursor);
SCTAB nMax = static_cast<SCTAB>(maTabs.size());
- ScMarkData::const_iterator itr = rMark.begin(), itrEnd = rMark.end();
+ ScMarkData::const_iterator itr = aMark.begin(), itrEnd = aMark.end();
for (; itr != itrEnd && *itr < nMax && !aData.bError; ++itr)
if (maTabs[*itr])
- maTabs[*itr]->UpdateSelectionFunction( aData,
- nStartCol, nStartRow, nEndCol, nEndRow, rMark );
+ maTabs[*itr]->UpdateSelectionFunction(aData, aMark);
//! rMark an UpdateSelectionFunction uebergeben !!!!!
diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx
index a927485..16ed68d 100644
--- a/sc/source/core/data/table3.cxx
+++ b/sc/source/core/data/table3.cxx
@@ -2308,30 +2308,15 @@ xub_StrLen ScTable::GetMaxNumberStringLen(
return 0;
}
-void ScTable::UpdateSelectionFunction( ScFunctionData& rData,
- SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW nEndRow,
- const ScMarkData& rMark )
+void ScTable::UpdateSelectionFunction( ScFunctionData& rData, const ScMarkData& rMark )
{
- // Cursor neben einer Markierung nicht beruecksichtigen:
- //! nur noch MarkData uebergeben, Cursorposition ggf. hineinselektieren!!!
- bool bSingle = ( rMark.IsMarked() || !rMark.IsMultiMarked() );
-
- // Mehrfachselektion:
-
- SCCOL nCol;
- if ( rMark.IsMultiMarked() )
- for (nCol=0; nCol<=MAXCOL && !rData.bError; nCol++)
- if ( !pColFlags || !ColHidden(nCol) )
- aCol[nCol].UpdateSelectionFunction( rMark, rData, *mpHiddenRows,
- bSingle && ( nCol >= nStartCol && nCol <= nEndCol ),
- nStartRow, nEndRow );
-
- // Einfachselektion (oder Cursor) nur wenn nicht negativ (und s.o.):
-
- if ( bSingle && !rMark.IsMarkNegative() )
- for (nCol=nStartCol; nCol<=nEndCol && !rData.bError; nCol++)
- if ( !pColFlags || !ColHidden(nCol) )
- aCol[nCol].UpdateAreaFunction( rData, *mpHiddenRows, nStartRow, nEndRow );
+ for (SCCOL nCol = 0; nCol <= MAXCOL && !rData.bError; ++nCol)
+ {
+ if (pColFlags && ColHidden(nCol))
+ continue;
+
+ aCol[nCol].UpdateSelectionFunction(rMark, rData, *mpHiddenRows);
+ }
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
More information about the Libreoffice-commits
mailing list