From 87eeac4e15b51779522f86aafe72673dcab18310 Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Tue, 30 Aug 2016 04:02:00 +0100 Subject: [PATCH] refs #710 Simple algorithms to replace several regular expressions. --- src/blackcore/vatsim/vatsimdatafilereader.cpp | 8 ++- .../vatsim/vatsimstatusfilereader.cpp | 7 ++- src/blackmisc/aviation/aircrafticaocode.cpp | 29 ++++----- src/blackmisc/aviation/aircrafticaocode.h | 2 +- src/blackmisc/aviation/airlineicaocode.cpp | 9 +-- src/blackmisc/aviation/airporticaocode.cpp | 5 +- src/blackmisc/aviation/callsign.cpp | 19 +----- .../xplane/aircraftmodelloaderxplane.cpp | 9 +-- src/blackmisc/stringutils.cpp | 14 ++++- src/blackmisc/stringutils.h | 62 +++++++++++++++++++ tests/blackmisc/testblackmiscmain.cpp | 5 ++ tests/blackmisc/teststringutils.cpp | 61 ++++++++++++++++++ tests/blackmisc/teststringutils.h | 45 ++++++++++++++ 13 files changed, 218 insertions(+), 57 deletions(-) create mode 100644 tests/blackmisc/teststringutils.cpp create mode 100644 tests/blackmisc/teststringutils.h diff --git a/src/blackcore/vatsim/vatsimdatafilereader.cpp b/src/blackcore/vatsim/vatsimdatafilereader.cpp index 10c930435..d1e74db41 100644 --- a/src/blackcore/vatsim/vatsimdatafilereader.cpp +++ b/src/blackcore/vatsim/vatsimdatafilereader.cpp @@ -230,7 +230,7 @@ namespace BlackCore CLogMessage(this).info("VATSIM file has same content, skipped"); return; } - const QStringList lines = dataFileData.split(QRegExp("[\r\n]"), QString::SkipEmptyParts); + const QList lines = splitLinesRefs(dataFileData); if (lines.isEmpty()) { return; } // build on local vars for thread safety @@ -243,7 +243,9 @@ namespace BlackCore QStringList clientSectionAttributes; Section section = SectionNone; - for (const QString &cl : lines) + + QString currentLine; // declared outside of the for loop, to amortize the cost of allocation + for (QStringRef clRef : lines) { if (this->isAbandoned()) { @@ -253,7 +255,7 @@ namespace BlackCore } // parse lines - QString currentLine(cl.trimmed()); + currentLine = clRef.toString().trimmed(); if (currentLine.isEmpty()) continue; if (currentLine.startsWith(";")) { diff --git a/src/blackcore/vatsim/vatsimstatusfilereader.cpp b/src/blackcore/vatsim/vatsimstatusfilereader.cpp index f593de709..d14b287e3 100644 --- a/src/blackcore/vatsim/vatsimstatusfilereader.cpp +++ b/src/blackcore/vatsim/vatsimstatusfilereader.cpp @@ -102,14 +102,15 @@ namespace BlackCore nwReply->close(); // close asap if (dataFileData.isEmpty()) return; - const QStringList lines = dataFileData.split(QRegExp("[\r\n]"), QString::SkipEmptyParts); + const QList lines = splitLinesRefs(dataFileData); if (lines.isEmpty()) { return; } CUrlList dataFileUrls; CUrlList serverFileUrls; CUrlList metarFileUrls; - for (const QString &cl : lines) + QString currentLine; // declared outside of the for loop, to amortize the cost of allocation + for (QStringRef clRef : lines) { if (this->isAbandoned()) { @@ -119,7 +120,7 @@ namespace BlackCore } // parse lines - const QString currentLine(cl.trimmed()); + currentLine = clRef.toString().trimmed(); if (currentLine.isEmpty()) { continue; } if (currentLine.startsWith(";")) { continue; } if (!currentLine.contains("=")) { continue; } diff --git a/src/blackmisc/aviation/aircrafticaocode.cpp b/src/blackmisc/aviation/aircrafticaocode.cpp index de7b475b8..0dd8b9606 100644 --- a/src/blackmisc/aviation/aircrafticaocode.cpp +++ b/src/blackmisc/aviation/aircrafticaocode.cpp @@ -450,11 +450,8 @@ namespace BlackMisc bool CAircraftIcaoCode::isValidDesignator(const QString &designator) { if (designator.length() < 2 || designator.length() > 5) { return false; } - - static QThreadStorage tsRegex; - if (! tsRegex.hasLocalData()) { tsRegex.setLocalData(QRegularExpression("^[A-Z]+[A-Z0-9]*$")); } - const QRegularExpression ®exp = tsRegex.localData(); - return (regexp.match(designator).hasMatch()); + if (!designator[0].isUpper()) { return false; } + return !containsChar(designator, [](QChar c) { return !c.isUpper() && !c.isDigit(); }); } bool CAircraftIcaoCode::isValidCombinedType(const QString &combinedType) @@ -462,11 +459,14 @@ namespace BlackMisc if (combinedType.length() != 3) { return false; } // Amphibian, Glider, Helicopter, Seaplane, Landplane, Tilt wing + static const QString validDescriptions = "AGHSLT"; // Electric, Jet, Piston, Turpoprop - static QThreadStorage tsRegex; - if (! tsRegex.hasLocalData()) { tsRegex.setLocalData(QRegularExpression("^[AGHSLT][0-9][EJPT]$")); } - const QRegularExpression ®exp = tsRegex.localData(); - return (regexp.match(combinedType).hasMatch()); + static const QString validEngines = "EJPT"; + + if (!validDescriptions.contains(combinedType[0])) { return false; } + if (!combinedType[1].isDigit()) { return false; } + if (!validEngines.contains(combinedType[2])) { return false; } + return true; } bool CAircraftIcaoCode::isValidWtc(const QString &candidate) @@ -491,16 +491,11 @@ namespace BlackMisc return s; } - QString CAircraftIcaoCode::normalizeDesignator(const QString candidate) + QString CAircraftIcaoCode::normalizeDesignator(const QString &candidate) { QString n(candidate.trimmed().toUpper()); - if (n.contains(' ')) { n = n.left(n.indexOf(' ')); } // cutoff as first space - if (n.isEmpty()) { return n; } - - static QThreadStorage tsRegex; - if (! tsRegex.hasLocalData()) { tsRegex.setLocalData(QRegularExpression("[^a-zA-Z\\d\\s]")); } - const QRegularExpression ®exp = tsRegex.localData(); - return n.remove(regexp); + n = n.left(indexOfChar(n, [](QChar c) { return c.isSpace(); })); + return removeChars(n, [](QChar c) { return !c.isLetterOrNumber(); }); } QStringList CAircraftIcaoCode::alternativeCombinedCodes(const QString &combinedCode) diff --git a/src/blackmisc/aviation/aircrafticaocode.h b/src/blackmisc/aviation/aircrafticaocode.h index 05af2a611..20cdee3da 100644 --- a/src/blackmisc/aviation/aircrafticaocode.h +++ b/src/blackmisc/aviation/aircrafticaocode.h @@ -259,7 +259,7 @@ namespace BlackMisc static const QStringList &getSpecialDesignators(); //! Normalize designator, remove illegal characters - static QString normalizeDesignator(const QString candidate); + static QString normalizeDesignator(const QString &candidate); //! Create relaxed combined codes, e.g "L2J" -> "L3J", ... static QStringList alternativeCombinedCodes(const QString &combinedCode); diff --git a/src/blackmisc/aviation/airlineicaocode.cpp b/src/blackmisc/aviation/airlineicaocode.cpp index 66c7c6576..e806aa4ef 100644 --- a/src/blackmisc/aviation/airlineicaocode.cpp +++ b/src/blackmisc/aviation/airlineicaocode.cpp @@ -301,13 +301,8 @@ namespace BlackMisc QString CAirlineIcaoCode::normalizeDesignator(const QString candidate) { QString n(candidate.trimmed().toUpper()); - if (n.contains(' ')) { n = n.left(n.indexOf(' ')); } // cutoff at first space - if (n.isEmpty()) { return n; } - - static QThreadStorage tsRegex; - if (! tsRegex.hasLocalData()) { tsRegex.setLocalData(QRegularExpression("[^a-zA-Z\\d\\s]")); } - const QRegularExpression ®exp = tsRegex.localData(); - return n.remove(regexp); + n = n.left(indexOfChar(n, [](QChar c) { return c.isSpace(); })); + return removeChars(n, [](QChar c) { return !c.isLetterOrNumber(); }); } QString CAirlineIcaoCode::getCombinedStringWithKey() const diff --git a/src/blackmisc/aviation/airporticaocode.cpp b/src/blackmisc/aviation/airporticaocode.cpp index 4f897ca12..106819003 100644 --- a/src/blackmisc/aviation/airporticaocode.cpp +++ b/src/blackmisc/aviation/airporticaocode.cpp @@ -31,9 +31,8 @@ namespace BlackMisc { QString code = icaoCode.trimmed().toUpper(); if (code.length() != 4) return ""; - QRegularExpression reg("^[A-Z0-9]{4}$"); - auto match = reg.match(code); - return match.hasMatch() ? code : QString(); + if (containsChar(code, [](QChar c) { return !c.isLetterOrNumber(); })) { return ""; } + return code; } bool CAirportIcaoCode::isValidIcaoDesignator(const QString &icaoCode) diff --git a/src/blackmisc/aviation/callsign.cpp b/src/blackmisc/aviation/callsign.cpp index 2fe7ade55..63acb2bdf 100644 --- a/src/blackmisc/aviation/callsign.cpp +++ b/src/blackmisc/aviation/callsign.cpp @@ -32,10 +32,7 @@ namespace BlackMisc QString CCallsign::unifyCallsign(const QString &callsign) { - QString unified = callsign.toUpper(); - // allow A-Z, 0-9, _, but no spaces - unified = unified.remove(QRegExp("[^A-Z\\d_]")); - return unified; + return removeChars(callsign.toUpper(), [](QChar c) { return !c.isLetterOrNumber() && c != '_'; }); } const CIcon &CCallsign::convertToIcon(const CCallsign &callsign) @@ -227,12 +224,7 @@ namespace BlackMisc bool CCallsign::isValidAircraftCallsign(const QString &callsign) { if (callsign.length() < 2 || callsign.length() > 10) { return false; } - - // We allow all number callsigns - static QThreadStorage tsRegex; - if (! tsRegex.hasLocalData()) { tsRegex.setLocalData(QRegularExpression("^[A-Z0-9]*$")); } - const QRegularExpression ®exp = tsRegex.localData(); - return (regexp.match(callsign).hasMatch()); + return !containsChar(callsign, [](QChar c) { return !c.isUpper() && !c.isDigit(); }); } bool CCallsign::isValidAircraftCallsign(const CCallsign &callsign) @@ -243,12 +235,7 @@ namespace BlackMisc bool CCallsign::isValidAtcCallsign(const QString &callsign) { if (callsign.length() < 2 || callsign.length() > 10) { return false; } - - // We allow all number callsigns - static QThreadStorage tsRegex; - if (! tsRegex.hasLocalData()) { tsRegex.setLocalData(QRegularExpression("^[A-Z0-9_]*$")); } - const QRegularExpression ®exp = tsRegex.localData(); - return (regexp.match(callsign).hasMatch()); + return !containsChar(callsign, [](QChar c) { return !c.isUpper() && !c.isDigit(); }); } bool CCallsign::isValidAtcCallsign(const CCallsign &callsign) diff --git a/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.cpp b/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.cpp index b281314c4..ba2f4af90 100644 --- a/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.cpp +++ b/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.cpp @@ -18,6 +18,7 @@ #include "blackmisc/simulation/xplane/aircraftmodelloaderxplane.h" #include "blackmisc/simulation/xplane/xplaneutil.h" #include "blackmisc/statusmessage.h" +#include "blackmisc/stringutils.h" #include "blackmisc/worker.h" #include @@ -386,7 +387,7 @@ namespace BlackMisc // Version number. QString versionLine = readLineFrom(ts); if (versionLine.isNull()) { return false; } - QString version = versionLine.split(QRegularExpression("\\s"), QString::SkipEmptyParts).at(0); + QString version = splitStringRefs(versionLine, [](QChar c) { return c.isSpace(); }).value(0).toString(); // For version 7, there is another line 'obj' if (version == "700") { readLineFrom(ts); } @@ -394,7 +395,7 @@ namespace BlackMisc // Texture QString textureLine = readLineFrom(ts); if (textureLine.isNull()) { return false; } - QString texture = textureLine.split(QRegularExpression("\\s"), QString::SkipEmptyParts).at(0); + QString texture = splitStringRefs(textureLine, [](QChar c) { return c.isSpace(); }).value(0).toString(); objFile.close(); @@ -570,7 +571,7 @@ namespace BlackMisc { ++lineNum; QString line = in.readLine(); - auto tokens = line.split(QRegularExpression("\\s+")); + auto tokens = splitString(line, [](QChar c) { return c.isSpace(); }); if (!tokens.empty()) { auto it = commands.find(tokens[0]); @@ -614,7 +615,7 @@ namespace BlackMisc { ++lineNum; QString line = in.readLine(); - auto tokens = line.split(QRegularExpression("\\s+"), QString::SkipEmptyParts); + auto tokens = splitString(line, [](QChar c) { return c.isSpace(); }); if (!tokens.empty()) { auto it = commands.find(tokens[0]); diff --git a/src/blackmisc/stringutils.cpp b/src/blackmisc/stringutils.cpp index 0e362c6e9..c06e04190 100644 --- a/src/blackmisc/stringutils.cpp +++ b/src/blackmisc/stringutils.cpp @@ -16,6 +16,16 @@ namespace BlackMisc { + QList splitLinesRefs(const QString &s) + { + return splitStringRefs(s, [](QChar c) { return c == '\n' || c == '\r'; }); + } + + QStringList splitLines(const QString &s) + { + return splitString(s, [](QChar c) { return c == '\n' || c == '\r'; }); + } + QString boolToOnOff(bool v, bool i18n) { Q_UNUSED(i18n); @@ -166,9 +176,7 @@ namespace BlackMisc QString simplifyNameForSearch(const QString &name) { - static const QRegularExpression reg("[^A-Z]"); - const QString r = name.toUpper().remove(reg); - return r; + return removeChars(name.toUpper(), [](QChar c) { return !c.isUpper(); }); } } diff --git a/src/blackmisc/stringutils.h b/src/blackmisc/stringutils.h index 5423b8d12..3d4f7aaca 100644 --- a/src/blackmisc/stringutils.h +++ b/src/blackmisc/stringutils.h @@ -13,22 +13,84 @@ #define BLACKMISC_STRINGUTILS_H #include "blackmisc/blackmiscexport.h" +#include "blackmisc/range.h" #include #include #include +#include #include #include +#include #include #include #include #include +#include template class QMap; //! Free functions in BlackMisc namespace BlackMisc { + //! Return a string with characters removed that match the given predicate. + template QString removeChars(const QString &s, F predicate) + { + QString result; + std::copy_if(s.begin(), s.end(), std::back_inserter(result), [=](auto c) { return !predicate(c); }); + return result; + } + + //! True if any character in the string matches the given predicate. + template bool containsChar(const QString &s, F predicate) + { + return std::any_of(s.begin(), s.end(), predicate); + } + + //! Index of first character in the string matching the given predicate, or -1 if not found. + template int indexOfChar(const QString &s, F predicate) + { + auto it = std::find_if(s.begin(), s.end(), predicate); + if (it == s.end()) { return -1; } + return std::distance(s.begin(), it); + } + + //! Split a string into multiple strings, using a predicate function to identify the split points. + //! \warning The returned refs are only valid during the lifetime of the original string. + template QList splitStringRefs(const QString &s, F predicate) + { + QList result; + auto notPredicate = [=](auto c) { return !predicate(c); }; + auto begin = s.begin(); + while (true) + { + begin = std::find_if(begin, s.end(), notPredicate); + if (begin == s.end()) { return result; } + auto end = std::find_if(begin, s.end(), predicate); + result.push_back(QStringRef(&s, std::distance(s.begin(), begin), std::distance(begin, end))); + begin = end; + } + } + + //! Split a string into multiple lines. Blank lines are skipped. + //! \warning The returned refs are only valid during the lifetime of the original string. + BLACKMISC_EXPORT QList splitLinesRefs(const QString &s); + + //! It would be risky to call splitStringRefs with an rvalue, so forbid it. + template void splitStringRefs(const QString &&, F) = delete; + + //! It would be risky to call splitLinesRefs with an rvalue, so forbid it. + void splitLinesRefs(const QString &&) = delete; + + //! Split a string into multiple strings, using a predicate function to identify the split points. + template QStringList splitString(const QString &s, F predicate) + { + return makeRange(splitStringRefs(s, predicate)).transform([](QStringRef sr) { return sr.toString(); }); + } + + //! Split a string into multiple lines. Blank lines are skipped. + BLACKMISC_EXPORT QStringList splitLines(const QString &s); + //! A map converted to string template QString qmapToString(const QMap &map) { diff --git a/tests/blackmisc/testblackmiscmain.cpp b/tests/blackmisc/testblackmiscmain.cpp index 0e6194729..2ce9842cf 100644 --- a/tests/blackmisc/testblackmiscmain.cpp +++ b/tests/blackmisc/testblackmiscmain.cpp @@ -24,6 +24,7 @@ #include "testmath.h" #include "testphysicalquantities.h" #include "testslot.h" +#include "teststringutils.h" #include "testvaluecache.h" #include "testvariantandmap.h" #include "testweather.h" @@ -74,6 +75,10 @@ namespace BlackMiscTest CTestSlot slotTests; status |= test.exec(&slotTests, "blackmisc_slot"); } + { + CTestStringUtils stringUtilTests; + status |= test.exec(&stringUtilTests, "blackmisc_stringutils"); + } { CTestValueCache valueCacheTests; status |= test.exec(&valueCacheTests, "blackmisc_valuecache"); diff --git a/tests/blackmisc/teststringutils.cpp b/tests/blackmisc/teststringutils.cpp new file mode 100644 index 000000000..e89536d85 --- /dev/null +++ b/tests/blackmisc/teststringutils.cpp @@ -0,0 +1,61 @@ +/* Copyright (C) 2013 + * swift Project Community / Contributors + * + * This file is part of swift project. It is subject to the license terms in the LICENSE file found in the top-level + * directory of this distribution and at http://www.swift-project.org/license.html. No part of swift project, + * including this file, may be copied, modified, propagated, or distributed except according to the terms + * contained in the LICENSE file. + */ + +//! \cond PRIVATE_TESTS + +/*! + * \file + * \ingroup testblackmisc + */ + +#include "teststringutils.h" +#include "blackmisc/stringutils.h" + +#include + +using namespace BlackMisc; + +namespace BlackMiscTest +{ + + void CTestStringUtils::testRemove() + { + QString s = "loUwP PeERr69"; + QVERIFY2(removeChars(s, [](QChar c) { return !c.isUpper(); }) == "UPPER", "Test removing characters by predicate"); + } + + void CTestStringUtils::testContains() + { + QString s = "string with a numb3r"; + QVERIFY2(containsChar(s, [](QChar c) { return c.isNumber(); }), "Test contains character by predicate"); + s = "string without a number"; + QVERIFY2(!containsChar(s, [](QChar c) { return c.isNumber(); }), "Test not contains character by predicate"); + } + + void CTestStringUtils::testIndexOf() + { + QString s = "string with a numb3r"; + QVERIFY2(indexOfChar(s, [](QChar c) { return c.isNumber(); }) == 18, "Test index of character by predicate"); + s = "string without a number"; + QVERIFY2(indexOfChar(s, [](QChar c) { return c.isNumber(); }) == -1, "Test not index of character by predicate"); + } + + void CTestStringUtils::testSplit() + { + QString s = "line one\nline two\r\nline three\n"; + QStringList lines = splitLines(s); + QVERIFY2(lines.size() == 3, "Test split string into lines: correct number of lines"); + QVERIFY2(lines[0] == "line one", "Test split string into lines: correct first line"); + QVERIFY2(lines[1] == "line two", "Test split string into lines: correct second line"); + QVERIFY2(lines[2] == "line three", "Test split string into lines: correct third line"); + } + +} + +//! \endcond diff --git a/tests/blackmisc/teststringutils.h b/tests/blackmisc/teststringutils.h new file mode 100644 index 000000000..1e5ac7ab0 --- /dev/null +++ b/tests/blackmisc/teststringutils.h @@ -0,0 +1,45 @@ +/* Copyright (C) 2014 + * swift project community / contributors + * + * This file is part of swift project. It is subject to the license terms in the LICENSE file found in the top-level + * directory of this distribution and at http://www.swift-project.org/license.html. No part of swift project, + * including this file, may be copied, modified, propagated, or distributed except according to the terms + * contained in the LICENSE file. + */ + +#ifndef BLACKMISCTEST_TESTSTRINGUTILS_H +#define BLACKMISCTEST_TESTSTRINGUTILS_H + +//! \cond PRIVATE_TESTS + +/*! + * \file + * \ingroup testblackmisc + */ + +#include + +namespace BlackMiscTest +{ + + //! Testing string utilities + class CTestStringUtils : public QObject + { + Q_OBJECT + + public: + //! Constructor + explicit CTestStringUtils(QObject *parent = nullptr) : QObject(parent) {} + + private slots: + void testRemove(); + void testContains(); + void testIndexOf(); + void testSplit(); + }; + +} + +//! \endcond + +#endif