D17731: Select curve only, when clicked near curve

[Alexander Semke]

asemke added a comment.


  This patch doesn't seem to contain the logic required for what you demonstrated in the video. It's only about the calculation of properties. Did you forget to add some other changes to this patch?
  
  Please be aware of our code style which is documented src/doc/coding_style.dox. The relevant parts for this patch are:
  
  - we don't use any brackets for one-line if-else statements
  - we use spacing after 'if' and before the opening bracket  -  if (condition) {...
  
  You can also check the file admin/READMY.astyle to see how to quickly re-format the code according to LabPlot's code style.

INLINE COMMENTS

> Column.cpp:436
> +
> +	d->updateColumnProperties();
> +	d->columnPropertiesAvailable = true;

why do you update the column properties here? Why not to update only when asked for in Column::columnProperties() similar to how this is done for statistics in Column::statistics()?

> Column.cpp:453
> +
> +		d->updateColumnProperties();
> +		d->columnPropertiesAvailable = true;

same here.

> Column.cpp:470
> +
> +	d->updateColumnProperties();
> +	d->columnPropertiesAvailable = true;

same here.

> Column.cpp:487
> +
> +		d->updateColumnProperties();
> +		d->columnPropertiesAvailable = true;

same here.

> Column.cpp:713
> +	d->updateColumnProperties();
> +	d->columnPropertiesAvailable = true;
> +

this part should't be required. It's enough to set d->columnPropertiesAvailable = false; below and to update the properties on demand in Column::columnProperties().

> ColumnPrivate.cpp:1252
> +
> +	QByteArray columnName = name().toLocal8Bit();
> +

this variable is only used in the DEBUG output below where name() can be used directly.

REPOSITORY
  R262 LabPlot

REVISION DETAIL
  https://phabricator.kde.org/D17731

To: Murmele, asemke
Cc: yurchor, kde-edu, Murmele, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20181222/ac217f5a/attachment-0001.html>