Skip to content

Commit 968ebc6

Browse files
jgeudensjgeudens
committed
Refactor expressionparser
1 parent 4dfd93e commit 968ebc6

5 files changed

Lines changed: 58 additions & 41 deletions

File tree

‎src/datahandling/expressionparser.cpp‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,33 @@
66

77
const QString ExpressionParser::_cRegisterFunctionTemplate = "r(%1%2)";
88

9-
ExpressionParser::ExpressionParser(QStringList& expressions)
9+
ExpressionParser::ExpressionParser(const QStringList& expressions) : _findRegRegex(ExpressionRegex::cMatchRegister)
1010
{
11-
_findRegRegex.setPattern(ExpressionRegex::cMatchRegister);
12-
_findRegRegex.optimize();
13-
1411
parseExpressions(expressions);
1512
}
1613

17-
void ExpressionParser::dataPoints(QList<DataPoint>& dataPointList)
14+
/*!
15+
* \brief Returns the deduplicated list of data points found during parsing.
16+
*/
17+
QList<DataPoint> ExpressionParser::dataPoints() const
1818
{
19-
dataPointList = _dataPoints;
19+
return _dataPoints;
2020
}
2121

22-
void ExpressionParser::processedExpressions(QStringList& expressionList)
22+
/*!
23+
* \brief Returns the list of processed expressions with register references replaced.
24+
*/
25+
QStringList ExpressionParser::processedExpressions() const
2326
{
24-
expressionList = _processedExpressions;
27+
return _processedExpressions;
2528
}
2629

27-
void ExpressionParser::parseExpressions(QStringList& expressions)
30+
void ExpressionParser::parseExpressions(const QStringList& expressions)
2831
{
2932
_processedExpressions.clear();
3033
_dataPoints.clear();
3134

32-
for (const QString& expression : std::as_const(expressions))
35+
for (const QString& expression : expressions)
3336
{
3437
_processedExpressions.append(processExpression(expression));
3538
}
@@ -42,8 +45,7 @@ QString ExpressionParser::processExpression(QString const& graphExpr)
4245

4346
if (!i.hasNext() && resultExpr.contains("$"))
4447
{
45-
auto msg = QString("Expression evaluation parsing failed (\"%1\")").arg(resultExpr);
46-
qCWarning(scopeComm) << msg;
48+
qCWarning(scopeComm) << QString("Expression evaluation parsing failed (\"%1\")").arg(resultExpr);
4749
}
4850

4951
while (i.hasNext())
@@ -65,7 +67,7 @@ QString ExpressionParser::processExpression(QString const& graphExpr)
6567
return resultExpr;
6668
}
6769

68-
bool ExpressionParser::processRegisterExpression(QString regExpr, DataPoint& dataPoint)
70+
bool ExpressionParser::processRegisterExpression(const QString& regExpr, DataPoint& dataPoint)
6971
{
7072
static const QRegularExpression regParseRegex(ExpressionRegex::cParseReg);
7173
QRegularExpressionMatch match = regParseRegex.match(regExpr);
@@ -96,20 +98,16 @@ bool ExpressionParser::processRegisterExpression(QString regExpr, DataPoint& dat
9698

9799
QString ExpressionParser::constructInternalRegisterFunction(DataPoint const& dataPoint, int size)
98100
{
99-
quint32 idx;
100-
if (_dataPoints.contains(dataPoint))
101-
{
102-
idx = _dataPoints.indexOf(dataPoint);
103-
}
104-
else
101+
qsizetype idx = _dataPoints.indexOf(dataPoint);
102+
if (idx < 0)
105103
{
106104
_dataPoints.append(dataPoint);
107105
idx = _dataPoints.size() - 1;
108106
}
109107

110108
/* Add dummy whitespaces to make sure positions in internal representations match visible expressions */
111-
QString regIdx = QString("%1").arg(idx);
112-
const int spacesCount = size - 3 - regIdx.size(); /* ignore ${} and idx string length */
109+
QString regIdx = QString::number(idx);
110+
const int spacesCount = qMax(0, size - 3 - regIdx.size()); /* ignore ${} and idx string length */
113111
QString spaces = QString(" ").repeated(spacesCount);
114112

115113
return QString(_cRegisterFunctionTemplate).arg(idx).arg(spaces);

‎src/datahandling/expressionparser.h‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,26 +5,25 @@
55
#include <QRegularExpression>
66
#include <QStringList>
77

8-
class ExpressionParser : public QObject
8+
class ExpressionParser
99
{
10-
Q_OBJECT
1110
public:
12-
explicit ExpressionParser(QStringList& expressions);
11+
explicit ExpressionParser(const QStringList& expressions);
1312

14-
void dataPoints(QList<DataPoint>& dataPointList);
15-
void processedExpressions(QStringList& expressionList);
13+
QList<DataPoint> dataPoints() const;
14+
QStringList processedExpressions() const;
1615

1716
private:
18-
void parseExpressions(QStringList& expressions);
17+
void parseExpressions(const QStringList& expressions);
1918

2019
QString processExpression(QString const& expr);
21-
bool processRegisterExpression(QString regExpr, DataPoint& dataPoint);
20+
bool processRegisterExpression(const QString& regExpr, DataPoint& dataPoint);
2221
QString constructInternalRegisterFunction(DataPoint const& dataPoint, int size);
2322

2423
QStringList _processedExpressions;
2524
QList<DataPoint> _dataPoints;
2625

27-
QRegularExpression _findRegRegex;
26+
const QRegularExpression _findRegRegex;
2827

2928
static const QString _cRegisterFunctionTemplate;
3029
};

‎src/datahandling/graphdatahandler.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
void GraphDataHandler::setupExpressions(GraphDataModel* pGraphDataModel, QList<DataPoint>& registerList)
1515
{
1616
QStringList exprList;
17-
QList<DataPoint> regList;
1817

1918
pGraphDataModel->activeGraphIndexList(&_activeIndexList);
2019
for (quint16 graphIdx : std::as_const(_activeIndexList))
@@ -23,12 +22,11 @@ void GraphDataHandler::setupExpressions(GraphDataModel* pGraphDataModel, QList<D
2322
}
2423

2524
ExpressionParser exprParser(exprList);
26-
exprParser.dataPoints(regList);
25+
const QList<DataPoint> regList = exprParser.dataPoints();
2726

2827
qCInfo(scopeComm) << "Active registers: " << DataPoint::dumpListToString(regList);
2928

30-
QStringList processedExpList;
31-
exprParser.processedExpressions(processedExpList);
29+
const QStringList processedExpList = exprParser.processedExpressions();
3230

3331
_valueParsers.clear();
3432

‎tests/datahandling/tst_expressionparser.cpp‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -236,16 +236,34 @@ void TestExpressionParser::manyRegisters()
236236
verifyParsing(input, expDataPoints, expExpressions);
237237
}
238238

239-
void TestExpressionParser::verifyParsing(QStringList exprList,
240-
QList<DataPoint>& expectedDataPoints,
241-
QStringList& expectedExpression)
239+
void TestExpressionParser::manyRegistersHighIndex()
242240
{
243-
QList<DataPoint> actualDataPoints;
244-
QStringList actualExpressionList;
241+
/* Verify qMax(0,...) guard: ${0} is 4 chars, but assigned index 10 (after 10 prior registers).
242+
* Unguarded: spacesCount = 4-3-2 = -1. Guarded: clamped to 0, producing r(10) instead of garbage. */
243+
auto input = QStringList() << "${1}" << "${2}" << "${3}" << "${4}" << "${5}"
244+
<< "${6}" << "${7}" << "${8}" << "${9}" << "${10}" << "${0}";
245+
auto expDataPoints = QList<DataPoint>()
246+
<< DataPoint("${1}", Device::cFirstDeviceId) << DataPoint("${2}", Device::cFirstDeviceId)
247+
<< DataPoint("${3}", Device::cFirstDeviceId) << DataPoint("${4}", Device::cFirstDeviceId)
248+
<< DataPoint("${5}", Device::cFirstDeviceId) << DataPoint("${6}", Device::cFirstDeviceId)
249+
<< DataPoint("${7}", Device::cFirstDeviceId) << DataPoint("${8}", Device::cFirstDeviceId)
250+
<< DataPoint("${9}", Device::cFirstDeviceId) << DataPoint("${10}", Device::cFirstDeviceId)
251+
<< DataPoint("${0}", Device::cFirstDeviceId);
252+
/* ${10} at idx=9: size=5, spacesCount=5-3-1=1 → r(9 )
253+
* ${0} at idx=10: size=4, spacesCount=qMax(0,4-3-2)=0 → r(10) */
254+
auto expExpressions = QStringList() << "r(0)" << "r(1)" << "r(2)" << "r(3)" << "r(4)"
255+
<< "r(5)" << "r(6)" << "r(7)" << "r(8)" << "r(9 )" << "r(10)";
245256

257+
verifyParsing(input, expDataPoints, expExpressions);
258+
}
259+
260+
void TestExpressionParser::verifyParsing(const QStringList& exprList,
261+
const QList<DataPoint>& expectedDataPoints,
262+
const QStringList& expectedExpression)
263+
{
246264
ExpressionParser parser(exprList);
247-
parser.dataPoints(actualDataPoints);
248-
parser.processedExpressions(actualExpressionList);
265+
const QList<DataPoint> actualDataPoints = parser.dataPoints();
266+
const QStringList actualExpressionList = parser.processedExpressions();
249267

250268
QVERIFY(actualDataPoints == expectedDataPoints);
251269
QVERIFY(actualExpressionList == expectedExpression);

‎tests/datahandling/tst_expressionparser.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ private slots:
3434
void constant();
3535
void manyRegisters();
3636

37-
void verifyParsing(QStringList exprList, QList<DataPoint>& expectedDataPoints, QStringList& expectedExpression);
37+
void manyRegistersHighIndex();
38+
39+
void verifyParsing(const QStringList& exprList,
40+
const QList<DataPoint>& expectedDataPoints,
41+
const QStringList& expectedExpression);
3842

3943
private:
4044
};

0 commit comments

Comments
 (0)