D13711: Support import of ROOT (CERN) TH1 histograms
Fabian Kristof
noreply at phabricator.kde.org
Mon Jun 25 11:17:50 UTC 2018
fkristof added inline comments.
INLINE COMMENTS
> ROOTFilter.cpp:168
> + QVector<void*> dataContainer;
> + int columnOffset = dataSource->prepareImport(dataContainer, importMode, imax - i0 + 1, 3,
> + {"Bin Center", "Content", "Error"},
columnOffSet can be const if you don't intend to change it.
> ROOTFilter.cpp:181
> + DEBUG("reading row " << i);
> + centerContainer[i - i0] = i > 0 && i < nbins - 1 ? 0.5 * (bins[i].lowedge + bins[i + 1].lowedge)
> + : i == 0 ? bins.front().lowedge // -infinity
I suggest you to group these conditions with parenthesis so it's easier to understand them.
> ROOTFilter.cpp:191
> +
> +void ROOTFilterPrivate::write(const QString & fileName, AbstractDataSource* dataSource) {
> + Q_UNUSED(fileName);
const QString & fileName
Extra space, remove it
> ROOTFilter.cpp:201
> + QStringList histList;
> + for (auto & hist : currentROOTHist->ListHistograms()) {
> + histList << QString::fromStdString(hist);
This should be
const auto&
> ROOTFilterPrivate.h:70
> + */
> + std::vector<std::string> ListHistograms() const;
> +
The code should follow our conventions. We use camelCase in the naming.
> ROOTOptionsWidget.cpp:72
> +
> + for (auto item: ui.twContent->selectedItems())
> + names << item->text();
const auto&
REPOSITORY
R262 LabPlot
REVISION DETAIL
https://phabricator.kde.org/D13711
To: croick, sgerlach, asemke, fkristof
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180625/54b5f186/attachment.html>
More information about the kde-edu
mailing list