D22010: Fix problems when automatically recalcuating columns

[Alexander Semke]

asemke added inline comments.

INLINE COMMENTS

> ColumnPrivate.cpp:903
> +	//disconnect(m_owner, SLOT(updateFormula())); // because as sender is THIS used!
> +	// This works:
> +	foreach (auto connection, m_connectionsUpdateFormula)

Ok. We can remove these comments.

> ColumnPrivate.cpp:904
> +	// This works:
> +	foreach (auto connection, m_connectionsUpdateFormula)
> +		disconnect(connection);

use range based for-loop here.

> ColumnPrivate.cpp:908
>  	if (autoUpdate) {
>  		QVector<Column*> columns;
>  		for (auto column : variableColumns)

this vector doesn't seem to be used. Maybe you can remove it as part of your next patch here.

> ColumnPrivate.cpp:962
> +		if (column->rowCount() > maxRowCount)
> +			maxRowCount = column->rowCount();
>  	}

check indentation here.

> ColumnPrivate.cpp:966
> +	Spreadsheet* spreadsheet = dynamic_cast<Spreadsheet*>(m_owner->parentAspect());
> +	if (spreadsheet) {
> +//		// use first column as reference for the minimal number of rows. For the

this check is obsolete since it must be always a Spreadsheet. You can add an assert here maybe.

> ColumnPrivate.h:142
>  	Column* m_owner;
> +	QList<QMetaObject::Connection> m_connectionsUpdateFormula;
>  };

QVector instead of QList. Not a big difference here, just a matter of convention in the code.

> Spreadsheet.cpp:676
> +		if (col->columnMode() != AbstractColumn::Numeric)
> +			col->setColumnMode(AbstractColumn::Numeric); // wieso??
> +

we allow to trigger this "Generate data from function values" also on integer columns. Since the results of the such calculations are floating point number usually, we convert here to Numeric.

> Spreadsheet.h:72
>  
> +	void updateColumnValues(QVector<Column*> columns, const QString expression, QStringList variableNames, QVector<Column *> variableColumns, QVector<QVector<double> *> xVectors, bool autoUpdate);
> +

we can work with references here.

> FunctionValuesDialog.cpp:367
>  	//resize the spreadsheet if one of the data vectors from other spreadsheet(s) has more elements then the current spreadsheet.
> -	if (m_spreadsheet->rowCount() < maxRowCount)
> +	if (m_spreadsheet->rowCount() > maxRowCount) // if to many rows, delete to save memory
>  		m_spreadsheet->setRowCount(maxRowCount);

what happens now if m_spreadsheet->rowCount() < maxRowCount?

> FunctionValuesDialog.cpp:372
>  	bool autoUpdate = (ui.chkAutoUpdate->checkState() == Qt::Checked);
> -	for (auto* col : m_columns) {
> -		if (col->columnMode() != AbstractColumn::Numeric)
> -			col->setColumnMode(AbstractColumn::Numeric);
> -
> -		col->setFormula(expression, variableNames, columns, autoUpdate);
> -		col->replaceValues(0, new_data);
> -	}
> +	m_spreadsheet->updateColumnValues(m_columns, expression, variableNames, columns, xVectors, autoUpdate);
>  

You removed the usage of ExpressinParser here. We can remove the include "EvaluationParser.h" in this file, too.

REPOSITORY
  R262 LabPlot

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

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