diff --git a/src/barchart/qbarseries.cpp b/src/barchart/qbarseries.cpp index 31117fe..b5709ab 100644 --- a/src/barchart/qbarseries.cpp +++ b/src/barchart/qbarseries.cpp @@ -167,25 +167,19 @@ bool QBarSeries::removeBarSets(QList sets) { Q_D(QBarSeries); - foreach (QBarSet* set, sets) { - if ((set == 0) || (!d->m_barSets.contains(set))) { - // Fail if any of the sets is null or isn't in m_barSets - return false; - } - if (sets.count(set) != 1) { - // Also fail if same set is more than once in given list. - return false; - } - } - + bool setsRemoved = false; foreach (QBarSet* set, sets) { if (d->m_barSets.contains(set)) { d->m_barSets.removeOne(set); QObject::disconnect(set->d_ptr.data(), SIGNAL(updatedBars()), d, SLOT(barsetChanged())); + setsRemoved = true; } } - emit d->restructuredBars(); - return true; + + if (setsRemoved) { + emit d->restructuredBars(); + } + return setsRemoved; } /*! diff --git a/test/auto/qbarseries/tst_qbarseries.cpp b/test/auto/qbarseries/tst_qbarseries.cpp index 52c6be6..b2c7d03 100644 --- a/test/auto/qbarseries/tst_qbarseries.cpp +++ b/test/auto/qbarseries/tst_qbarseries.cpp @@ -167,15 +167,32 @@ void tst_QBarSeries::appendBarSet() { QVERIFY(m_barseries->barsetCount() == 0); + bool ret = false; + + // Try adding barset QBarSet *barset = new QBarSet("testset"); - m_barseries->appendBarSet(barset); + ret = m_barseries->appendBarSet(barset); + QVERIFY(ret == true); QVERIFY(m_barseries->barsetCount() == 1); + // Try adding another set QBarSet *barset2 = new QBarSet("testset2"); - m_barseries->appendBarSet(barset2); + ret = m_barseries->appendBarSet(barset2); + + QVERIFY(ret == true); + QVERIFY(m_barseries->barsetCount() == 2); + // Try adding same set again + ret = m_barseries->appendBarSet(barset2); + QVERIFY(ret == false); QVERIFY(m_barseries->barsetCount() == 2); + + // Try adding null set + ret = m_barseries->appendBarSet(0); + QVERIFY(ret == false); + QVERIFY(m_barseries->barsetCount() == 2); + } void tst_QBarSeries::removeBarSet_data() @@ -187,10 +204,24 @@ void tst_QBarSeries::removeBarSet() int count = m_testSets.count(); QVERIFY(m_barseries_with_sets->barsetCount() == count); + // Try to remove null pointer (should not remove, should not crash) + bool ret = false; + ret = m_barseries_with_sets->removeBarSet(0); + QVERIFY(ret == false); + QVERIFY(m_barseries_with_sets->barsetCount() == count); + + // Try to remove invalid pointer (should not remove, should not crash) + ret = m_barseries_with_sets->removeBarSet((QBarSet*) (m_testSets.at(0) + 1) ); + QVERIFY(ret == false); + QVERIFY(m_barseries_with_sets->barsetCount() == count); + // remove some sets - m_barseries_with_sets->removeBarSet(m_testSets.at(2)); - m_barseries_with_sets->removeBarSet(m_testSets.at(3)); - m_barseries_with_sets->removeBarSet(m_testSets.at(4)); + ret = m_barseries_with_sets->removeBarSet(m_testSets.at(2)); + QVERIFY(ret == true); + ret = m_barseries_with_sets->removeBarSet(m_testSets.at(3)); + QVERIFY(ret == true); + ret = m_barseries_with_sets->removeBarSet(m_testSets.at(4)); + QVERIFY(ret == true); QVERIFY(m_barseries_with_sets->barsetCount() == 2); @@ -199,11 +230,13 @@ void tst_QBarSeries::removeBarSet() QVERIFY(verifysets.at(0) == m_testSets.at(0)); QVERIFY(verifysets.at(1) == m_testSets.at(1)); - // Try removing all sets again + // Try removing all sets again (should be ok, even if some sets have already been removed) + ret = false; for (int i=0; iremoveBarSet(m_testSets.at(i)); + ret |= m_barseries_with_sets->removeBarSet(m_testSets.at(i)); } + QVERIFY(ret == true); QVERIFY(m_barseries_with_sets->barsetCount() == 0); } @@ -222,7 +255,37 @@ void tst_QBarSeries::appendBarSets() sets.append(new QBarSet("testset")); } - m_barseries->appendBarSets(sets); + // Append new sets (should succeed, count should match the count of sets) + bool ret = false; + ret = m_barseries->appendBarSets(sets); + QVERIFY(ret == true); + QVERIFY(m_barseries->barsetCount() == count); + + // Append same sets again (should fail, count should remain same) + ret = m_barseries->appendBarSets(sets); + QVERIFY(ret == false); + QVERIFY(m_barseries->barsetCount() == count); + + // Try append empty list (should succeed, but count should remain same) + QList invalidList; + ret = m_barseries->appendBarSets(invalidList); + QVERIFY(ret == true); + QVERIFY(m_barseries->barsetCount() == count); + + // Try append list with one new and one existing set (should fail, count remains same) + invalidList.append(new QBarSet("ok set")); + invalidList.append(sets.at(0)); + ret = m_barseries->appendBarSets(invalidList); + QVERIFY(ret == false); + QVERIFY(m_barseries->barsetCount() == count); + + // Try append list with null pointers (should fail, count remains same) + QList invalidList2; + invalidList2.append(0); + invalidList2.append(0); + invalidList2.append(0); + ret = m_barseries->appendBarSets(invalidList2); + QVERIFY(ret == false); QVERIFY(m_barseries->barsetCount() == count); } @@ -236,21 +299,36 @@ void tst_QBarSeries::removeBarSets() int count = m_testSets.count(); QVERIFY(m_barseries_with_sets->barsetCount() == count); - // Try removing empty list of sets - QList empty; - m_barseries_with_sets->removeBarSets(empty); + // Try removing empty list of sets (should return false, since no barsets were removed) + bool ret = false; + QList invalidList; + ret = m_barseries_with_sets->removeBarSets(invalidList); + QVERIFY(ret == false); + QVERIFY(m_barseries_with_sets->barsetCount() == count); + + // Add some null pointers to list + invalidList.append(0); + invalidList.append(0); + invalidList.append(0); + + // Try removing null pointers from list (should return false, should not crash, should not remove anything) + ret = m_barseries_with_sets->removeBarSets(invalidList); + QVERIFY(ret == false); QVERIFY(m_barseries_with_sets->barsetCount() == count); - // remove all sets - m_barseries_with_sets->removeBarSets(m_testSets); + // remove all sets (should return true, since sets were removed) + ret = m_barseries_with_sets->removeBarSets(m_testSets); + QVERIFY(ret == true); QVERIFY(m_barseries_with_sets->barsetCount() == 0); - // Try removing empty list again - m_barseries_with_sets->removeBarSets(empty); + // Try removing invalid list again (should return false, since no barsets were removed) + ret = m_barseries_with_sets->removeBarSets(invalidList); + QVERIFY(ret == false); QVERIFY(m_barseries_with_sets->barsetCount() == 0); - // remove all sets again - m_barseries_with_sets->removeBarSets(m_testSets); + // remove all sets again (should return false, since barsets were already removed) + ret = m_barseries_with_sets->removeBarSets(m_testSets); + QVERIFY(ret == false); QVERIFY(m_barseries_with_sets->barsetCount() == 0); }