QCustomPlot Discussion and Comments

Several fixesReturn to overview

1. axis.cpp 1650
QList<QCPAbstractItem*> QCPAxis::items() const

for (int itemId=0; itemId<mParentPlot->mItems.size(); ++itemId)
{
    for (int posId=0; posId<positions.size(); ++itemId) // <====
    {
       
    }
}

should be
for (int itemId=0; itemId<mParentPlot->mItems.size(); ++itemId)
{
    for (int posId=0; posId<positions.size(); ++posId)
    {
       
    }
}

2. layoutelement-axisrect.cpp 502
same here, should be for (int posId=0; posId<positions.size(); ++posId)

3. item-tracer.cpp 342

QCPDataMap::const_iterator it = first;
it = mGraph->data()->lowerBound(mGraphKey);

I think code can be reduced:

QCPDataMap::const_iterator it = mGraph->data()->lowerBound(mGraphKey);


It looks to me like non-sense code right now since you've left no details.
I'm not going to say your suggestions are wrong, but could you please provide some details about why the actual QCP code is broken right now and what will your changes do?

1. axis.cpp 1650
QList<QCPAbstractItem*> QCPAxis::items() const

For example we have func:

void MainWindow::addLineOnAxisDoubleClick(QCPAxis *axis)
{
    QCPItemStraightLine * line = new QCPItemStraightLine(ui->customPlot);
    ui->customPlot->addItem(line);
    QList<QCPAbstractItem*> lst = axis->items();
}

...
for (int posId=0; posId<positions.size(); ++itemId)
{
  if (positions.at(posId)->keyAxis() == this || positions.at(posId)->valueAxis() == this)
  {
     result.append(mParentPlot->mItems.at(itemId));
     break;
   }
}
...

If axis is QCPAxis::atLeft or QCPAxis::atBottom everything will be ok, and if QCPAxis::atRight or QCPAxis::atTop will be infity loop in axis->items().

if (positions.at(posId)->keyAxis() == this || positions.at(posId)->valueAxis() == this) never will be fulfilled and for next iteration posId will remain same. Infity loop.

With my previous proposal axis->items() will return empty list if QCPAxis::atRight or QCPAxis::atTop.

Therefore I propose:

...
for (int posId=0; posId<positions.size(); ++posId)
{
  QCPAxisRect * axisRect = positions.at(posId)->axisRect();
  if (positions.at(posId)->keyAxis() == this ||
      positions.at(posId)->valueAxis() == this) ||
      axisRect->axis(QCPAxis::atRight) == this ||
      axisRect->axis(QCPAxis::atTop) == this
  {
     result.append(mParentPlot->mItems.at(itemId));
     break;
   }
}
...

2. For second issue I not yet invented test case.

Hi maksqwe, thanks alot for the feedback, those are all valid issues that you mention!
They will be fixed in the next patch release soon.

I have to ask: Did you use a static analysis tool to spot them? Because cppcheck didn't notice.

Yes, I used PVS-Studio trial version. Great tool but not free.

And small cosmetic issue in common qcustomplot.h file. Double declarations of QCPAbstractItem:

class QCPAbstractItem; <<===== line 70
class QCPItemPosition;
class QCPLayer;
class QCPPlotTitle;
class QCPLegend;
class QCPAbstractLegendItem;
class QCPAbstractItem; <<===== line 76

Thank you for your details, they make more sense to me now ;)
Until a fix is released, I guess I can also manually patch QCP, right?