Clarify the code a little more.

Change-Id: Idc0ff4913695ade9543cd30511aff61d24e7c495

3 files changed, 63 insertions, 38 deletions

diff --git a/chart2/source/controller/dialogs/DataBrowserModel.cxx b/chart2/source/controller/dialogs/DataBrowserModel.cxx
index 49dfec30d64f..e2bff5b99691 100644
--- a/chart2/source/controller/dialogs/DataBrowserModel.cxx
+++ b/chart2/source/controller/dialogs/DataBrowserModel.cxx
@@ -186,9 +186,10 @@ struct lcl_RepresentationsOfLSeqMatch : public ::std::unary_function< Reference<
     {}
     bool operator() ( const Reference< chart2::data::XLabeledDataSequence > & xLSeq )
     {
-        return (xLSeq.is() &&
-                xLSeq->getValues().is() &&
-                (xLSeq->getValues()->getSourceRangeRepresentation() == m_aValuesRep ));
+        if (!xLSeq.is() || !xLSeq->getValues().is())
+            return false;
+
+        return xLSeq->getValues()->getSourceRangeRepresentation() == m_aValuesRep;
     }
 private:
     OUString m_aValuesRep;
@@ -475,42 +476,55 @@ void DataBrowserModel::removeDataSeriesOrComplexCategoryLevel( sal_Int32 nAtColu
 
     //delete sequences from internal data provider that are not used anymore
     //but do not delete sequences that are still in use by the remaining series
+
     Reference< chart2::XInternalDataProvider > xDataProvider( m_apDialogModel->getDataProvider(), uno::UNO_QUERY );
-    Reference< chart2::data::XDataSource > xSourceOfDeletedSeries( xSeries, uno::UNO_QUERY );
-    if( xDataProvider.is() && xSourceOfDeletedSeries.is())
+    Reference< chart2::data::XDataSource > xSourceOfDeleted( xSeries, uno::UNO_QUERY );
+    if (!xDataProvider.is() || !xSourceOfDeleted.is())
     {
-        ::std::vector< sal_Int32 > aSequenceIndexesToDelete;
-        Sequence< Reference< chart2::data::XLabeledDataSequence > > aSequencesOfDeletedSeries( xSourceOfDeletedSeries->getDataSequences() );
-        Reference< chart2::XDataSeriesContainer > xSeriesCnt( getHeaderForSeries( xSeries ).m_xChartType, uno::UNO_QUERY );
-        if( xSeriesCnt.is())
-        {
-            Reference< chart2::data::XDataSource > xRemainingDataSource( DataSeriesHelper::getDataSource( xSeriesCnt->getDataSeries() ) );
-            if( xRemainingDataSource.is() )
-            {
-                ::std::vector< Reference< chart2::data::XLabeledDataSequence > > aRemainingSeq( ContainerHelper::SequenceToVector( xRemainingDataSource->getDataSequences() ) );
-                for( sal_Int32 i=0; i<aSequencesOfDeletedSeries.getLength(); ++i )
-                {
-                    ::std::vector< Reference< chart2::data::XLabeledDataSequence > >::const_iterator aHitIt(
-                        ::std::find_if( aRemainingSeq.begin(), aRemainingSeq.end(),
-                            lcl_RepresentationsOfLSeqMatch( aSequencesOfDeletedSeries[i] )));
-                    // if not used by the remaining series this sequence can be deleted
-                    if( aHitIt == aRemainingSeq.end() )
-                        aSequenceIndexesToDelete.push_back( lcl_getValuesRepresentationIndex( aSequencesOfDeletedSeries[i] ) );
-                }
-            }
-        }
+        // Something went wrong.  Bail out.
+        updateFromModel();
+        return;
+    }
 
-        // delete unnecessary sequences of the internal data
-        // iterate using greatest index first, so that deletion does not
-        // shift other sequences that will be deleted later
-        ::std::sort( aSequenceIndexesToDelete.begin(), aSequenceIndexesToDelete.end());
-        for( ::std::vector< sal_Int32 >::reverse_iterator aIt(
-                 aSequenceIndexesToDelete.rbegin()); aIt != aSequenceIndexesToDelete.rend(); ++aIt )
-        {
-            if( *aIt != -1 )
-                xDataProvider->deleteSequence( *aIt );
-        }
+    Reference<chart2::XDataSeriesContainer> xSeriesCnt(
+        getHeaderForSeries(xSeries).m_xChartType, uno::UNO_QUERY);
+    if (!xSeriesCnt.is())
+    {
+        // Unexpected happened.  Bail out.
+        updateFromModel();
+        return;
     }
+
+    // Collect all the remaining data sequences in the same chart type. The
+    // deleted data series is already gone by this point.
+    std::vector<Reference<chart2::data::XLabeledDataSequence> > aAllDataSeqs =
+        DataSeriesHelper::getAllDataSequences(xSeriesCnt->getDataSeries());
+
+    // Check if the sequences to be deleted are still referenced by any of
+    // the other data series.  If not, mark them for deletion.
+    std::vector<sal_Int32> aSequenceIndexesToDelete;
+    Sequence<Reference<chart2::data::XLabeledDataSequence> > aSequencesOfDeleted = xSourceOfDeleted->getDataSequences();
+    for (sal_Int32 i = 0; i < aSequencesOfDeleted.getLength(); ++i)
+    {
+        std::vector<Reference<chart2::data::XLabeledDataSequence> >::const_iterator aHitIt(
+            ::std::find_if( aAllDataSeqs.begin(), aAllDataSeqs.end(),
+                lcl_RepresentationsOfLSeqMatch( aSequencesOfDeleted[i] )));
+        // if not used by the remaining series this sequence can be deleted
+        if( aHitIt == aAllDataSeqs.end() )
+            aSequenceIndexesToDelete.push_back( lcl_getValuesRepresentationIndex( aSequencesOfDeleted[i] ) );
+    }
+
+    // delete unnecessary sequences of the internal data
+    // iterate using greatest index first, so that deletion does not
+    // shift other sequences that will be deleted later
+    ::std::sort( aSequenceIndexesToDelete.begin(), aSequenceIndexesToDelete.end());
+    for( ::std::vector< sal_Int32 >::reverse_iterator aIt(
+             aSequenceIndexesToDelete.rbegin()); aIt != aSequenceIndexesToDelete.rend(); ++aIt )
+    {
+        if( *aIt != -1 )
+            xDataProvider->deleteSequence( *aIt );
+    }
+
     updateFromModel();
 }
 
diff --git a/chart2/source/inc/DataSeriesHelper.hxx b/chart2/source/inc/DataSeriesHelper.hxx
index 9792b713b736..13ea4bce7dbd 100644
--- a/chart2/source/inc/DataSeriesHelper.hxx
+++ b/chart2/source/inc/DataSeriesHelper.hxx
@@ -75,6 +75,11 @@ OOO_DLLPUBLIC_CHARTTOOLS ::std::vector<
                                const OUString& aRole,
                                bool bMatchPrefix = false );
 
+OOO_DLLPUBLIC_CHARTTOOLS
+std::vector<css::uno::Reference<css::chart2::data::XLabeledDataSequence> >
+getAllDataSequences(
+    const css::uno::Sequence<css::uno::Reference<css::chart2::XDataSeries> >& aSeries );
+
 /** Retrieves all data sequences found in the given data series and puts them
     into a data source.  The order of sequences will match the order of the data
     series.
diff --git a/chart2/source/tools/DataSeriesHelper.cxx b/chart2/source/tools/DataSeriesHelper.cxx
index a3b6cd5f4efd..3066402a76eb 100644
--- a/chart2/source/tools/DataSeriesHelper.cxx
+++ b/chart2/source/tools/DataSeriesHelper.cxx
@@ -237,8 +237,8 @@ Reference< chart2::data::XLabeledDataSequence >
     return aResultVec;
 }
 
-Reference< chart2::data::XDataSource >
-    getDataSource( const Sequence< Reference< chart2::XDataSeries > > & aSeries )
+std::vector<Reference<css::chart2::data::XLabeledDataSequence> >
+getAllDataSequences( const uno::Sequence<uno::Reference<chart2::XDataSeries> >& aSeries )
 {
     ::std::vector< Reference< chart2::data::XLabeledDataSequence > > aSeqVec;
 
@@ -253,8 +253,14 @@ Reference< chart2::data::XDataSource >
         }
     }
 
+    return aSeqVec;
+}
+
+Reference< chart2::data::XDataSource >
+    getDataSource( const Sequence< Reference< chart2::XDataSeries > > & aSeries )
+{
     return Reference< chart2::data::XDataSource >(
-        new DataSource( ContainerHelper::ContainerToSequence( aSeqVec )));
+        new DataSource(ContainerHelper::ContainerToSequence(getAllDataSequences(aSeries))));
 }
 
 namespace