D22200: Column observer
Alexander Semke
noreply at phabricator.kde.org
Wed Jul 10 07:49:36 BST 2019
asemke added inline comments.
INLINE COMMENTS
> Project.cpp:202
> + * \brief Project::descriptionChanged
> + * When the column is created, it gets a random name and is eventually not connected to any curve.
> + * When changing the name it can match a curve and should than be connected to the curve.
move this comment inside the function definition. This is the comment for the handling of columns only and not for the whole function which does a bit more.
> Project.cpp:221
> + // xColumnPath is set in setXColumn. Same for the yColumn.
> + for (auto* curve : curves) {
> + auto* analysisCurve = dynamic_cast<XYAnalysisCurve*>(curve);
in this for loop we call several set-functions which will produce a new entry on the undo/redo-stack. Do we want to have these entries if we change the name of a column? I'd say no. Le's work with curve->setUndoAware(false);
curve->set*Column(column);
curve->setUndoAware(true);
> Project.cpp:276
> +
> + for (auto* curve : curves) {
> + auto* analysisCurve = dynamic_cast<XYAnalysisCurve*>(curve);
same here. Let's use setUndoAware(false) prior to setting the columns.
> Project.h:37
> class QString;
> +class ColumnObserver;
>
can be removed.
> XYCurveDock.cpp:556
>
> + checkCurveAvailability(m_curve, cbXColumn, "xColumn");
> + checkCurveAvailability(m_curve, cbYColumn, "yColumn");
wouldn't it be easier here to provide the column and the column path directly to this function like
checkCurveAvailability(cbXColumn, curve->xColumn(), curve->xColumnPath());
With this you'd avoid this cumbersome if-else logic in XYCurveDock::checkCurveAvailability().
REPOSITORY
R262 LabPlot
REVISION DETAIL
https://phabricator.kde.org/D22200
To: Murmele, asemke
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190710/5268a3f3/attachment-0001.html>
More information about the kde-edu
mailing list