From f0fe65e7690bc76f715f2987ac583ae077ca3c96 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Tue, 7 Mar 2023 23:21:05 -0600 Subject: [PATCH 1/2] App: Add IndexedName class Ported from development/toponaming. --- src/App/CMakeLists.txt | 2 + src/App/IndexedName.cpp | 127 ++++++++++++++++++++++++++++ src/App/IndexedName.h | 181 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 310 insertions(+) create mode 100644 src/App/IndexedName.cpp create mode 100644 src/App/IndexedName.h diff --git a/src/App/CMakeLists.txt b/src/App/CMakeLists.txt index 02ebe92de202..918ced0a6fb9 100644 --- a/src/App/CMakeLists.txt +++ b/src/App/CMakeLists.txt @@ -261,6 +261,7 @@ SET(FreeCADApp_CPP_SRCS ComplexGeoData.cpp ComplexGeoDataPyImp.cpp Enumeration.cpp + IndexedName.cpp Material.cpp MaterialPyImp.cpp Metadata.cpp @@ -277,6 +278,7 @@ SET(FreeCADApp_HPP_SRCS ColorModel.h ComplexGeoData.h Enumeration.h + IndexedName.h Material.h Metadata.h ) diff --git a/src/App/IndexedName.cpp b/src/App/IndexedName.cpp new file mode 100644 index 000000000000..e4bc448d4dd0 --- /dev/null +++ b/src/App/IndexedName.cpp @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +/**************************************************************************** + * Copyright (c) 2022 Zheng, Lei (realthunder) * + * * + * This file is part of FreeCAD. * + * * + * FreeCAD is free software: you can redistribute it and/or modify it * + * under the terms of the GNU Lesser General Public License as * + * published by the Free Software Foundation, either version 2.1 of the * + * License, or (at your option) any later version. * + * * + * FreeCAD is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with FreeCAD. If not, see * + * . * + * * + ***************************************************************************/ + +#include "PreCompiled.h" + +#ifndef _PreComp_ +# include +# include +#endif + +#include + +#include "IndexedName.h" + +using namespace Data; + +struct ByteArray +{ + ByteArray(const QByteArray& b) + :bytes(b) + {} + + ByteArray(const ByteArray& other) + :bytes(other.bytes) + {} + + ByteArray(ByteArray&& other) + :bytes(std::move(other.bytes)) + {} + + void mutate() const + { + QByteArray copy; + copy.append(bytes.constData(), bytes.size()); + bytes = copy; + } + + bool operator==(const ByteArray& other) const { + return bytes == other.bytes; + } + + mutable QByteArray bytes; +}; + +struct ByteArrayHasher +{ + std::size_t operator()(const ByteArray& bytes) const + { + return qHash(bytes.bytes); + } + + std::size_t operator()(const QByteArray& bytes) const + { + return qHash(bytes); + } +}; + +void IndexedName::set( + const char* name, + int len, + const std::vector& types, + bool allowOthers) +{ + static std::unordered_set NameSet; + + if (len < 0) + len = static_cast(std::strlen(name)); + int i; + for (i = len - 1; i >= 0; --i) { + if (name[i] < '0' || name[i]>'9') + break; + } + ++i; + this->index = std::atoi(name + i); + + for (int j = 0; j < i; ++j) { + if (name[j] == '_' + || (name[j] >= 'a' && name[j] <= 'z') + || (name[j] >= 'A' && name[j] <= 'Z')) + continue; + this->type = ""; + return; + } + + for (const char* type : types) { + int j = 0; + for (const char* n = name, *t = type; *n; ++n) { + if (*n != *t || j >= i) + break; + ++j; + ++t; + if (!*t) { + this->type = type; + return; + } + } + } + + if (allowOthers) { + auto res = NameSet.insert(QByteArray::fromRawData(name, i)); + if (res.second) + res.first->mutate(); + this->type = res.first->bytes.constData(); + } + else + this->type = ""; +} diff --git a/src/App/IndexedName.h b/src/App/IndexedName.h new file mode 100644 index 000000000000..3e6488293492 --- /dev/null +++ b/src/App/IndexedName.h @@ -0,0 +1,181 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +/**************************************************************************** + * Copyright (c) 2022 Zheng, Lei (realthunder) * + * * + * This file is part of FreeCAD. * + * * + * FreeCAD is free software: you can redistribute it and/or modify it * + * under the terms of the GNU Lesser General Public License as * + * published by the Free Software Foundation, either version 2.1 of the * + * License, or (at your option) any later version. * + * * + * FreeCAD is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with FreeCAD. If not, see * + * . * + * * + ***************************************************************************/ + + +#ifndef _IndexedName_h_ +#define _IndexedName_h_ + +#include +#include +#include +#include +#include + +#include + +#include "FCGlobal.h" + + +namespace Data +{ + +class AppExport IndexedName { +public: + explicit IndexedName(const char *name = nullptr, int _index = 0) + : index(0) + { + if (!name) + this->type = ""; + else { + set(name); + if (_index) + this->index = _index; + } + } + + IndexedName(const char *name, + const std::vector & types, + bool allowOthers=true) + { + set(name, -1, types, allowOthers); + } + + explicit IndexedName(const QByteArray & data) + { + set(data.constData(), data.size()); + } + + IndexedName(const IndexedName &other) + : type(other.type), index(other.index) + {} + + static IndexedName fromConst(const char *name, int index) { + IndexedName res; + res.type = name; + res.index = index; + return res; + } + + IndexedName & operator=(const IndexedName & other) + { + this->index = other.index; + this->type = other.type; + return *this; + } + + friend std::ostream & operator<<(std::ostream & s, const IndexedName & e) + { + s << e.type; + if (e.index > 0) + s << e.index; + return s; + } + + bool operator==(const IndexedName & other) const + { + return this->index == other.index + && (this->type == other.type + || std::strcmp(this->type, other.type)==0); + } + + IndexedName & operator+=(int offset) + { + this->index += offset; + assert(this->index >= 0); + return *this; + } + + IndexedName & operator++() + { + ++this->index; + return *this; + } + + IndexedName & operator--() + { + --this->index; + assert(this->index >= 0); + return *this; + } + + bool operator!=(const IndexedName & other) const + { + return !(this->operator==(other)); + } + + const char * toString(std::string & s) const + { + // Note! s is not cleared on purpose. + std::size_t offset = s.size(); + s += this->type; + if (this->index > 0) + s += std::to_string(this->index); + return s.c_str() + offset; + } + + int compare(const IndexedName & other) const + { + int res = std::strcmp(this->type, other.type); + if (res) + return res; + if (this->index < other.index) + return -1; + if (this->index > other.index) + return 1; + return 0; + } + + bool operator<(const IndexedName & other) const + { + return compare(other) < 0; + } + + char operator[](int index) const + { + return this->type[index]; + } + + const char * getType() const { return this->type; } + + int getIndex() const { return this->index; } + + void setIndex(int index) { assert(index>=0); this->index = index; } + + bool isNull() const { return !this->type[0]; } + + explicit operator bool() const { return !isNull(); } + +protected: + void set(const char *, + int len = -1, + const std::vector &types = {}, + bool allowOthers = true); + +private: + const char * type; + int index; +}; + +} + +#endif _IndexedName_h_ From cc9387aa3faceee74d3e2c073146b9ecba171168 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Tue, 7 Mar 2023 23:55:16 -0600 Subject: [PATCH 2/2] App: Clean up IndexedName and add tests Fixes the matching algorithm when provided a vector of existing names: The original algorithm was equivalent to a 'startswith' algorithm, when it should have been testing for exact and complete string equality. This also does some refactoring to rename and clarify variables and functions, simplifies some functions by using standard library calls when possible, and addresses various linter complaints. It also applies our current clang-format to the files. Co-authored-by: Ajinkya Dahale --- src/App/IndexedName.cpp | 129 ++++---- src/App/IndexedName.h | 255 ++++++++++++--- tests/src/App/CMakeLists.txt | 1 + tests/src/App/IndexedName.cpp | 595 ++++++++++++++++++++++++++++++++++ 4 files changed, 864 insertions(+), 116 deletions(-) create mode 100644 tests/src/App/IndexedName.cpp diff --git a/src/App/IndexedName.cpp b/src/App/IndexedName.cpp index e4bc448d4dd0..ebe9bf2ddbc6 100644 --- a/src/App/IndexedName.cpp +++ b/src/App/IndexedName.cpp @@ -2,6 +2,7 @@ /**************************************************************************** * Copyright (c) 2022 Zheng, Lei (realthunder) * + * Copyright (c) 2023 FreeCAD Project Association * * * * This file is part of FreeCAD. * * * @@ -21,6 +22,7 @@ * * ***************************************************************************/ +// NOLINTNEXTLINE #include "PreCompiled.h" #ifndef _PreComp_ @@ -28,100 +30,93 @@ # include #endif -#include - #include "IndexedName.h" using namespace Data; -struct ByteArray +/// Check whether the input character is an underscore or an ASCII letter a-Z or A-Z +inline bool isInvalidChar(char test) { - ByteArray(const QByteArray& b) - :bytes(b) - {} - - ByteArray(const ByteArray& other) - :bytes(other.bytes) - {} - - ByteArray(ByteArray&& other) - :bytes(std::move(other.bytes)) - {} - - void mutate() const - { - QByteArray copy; - copy.append(bytes.constData(), bytes.size()); - bytes = copy; - } - - bool operator==(const ByteArray& other) const { - return bytes == other.bytes; - } - - mutable QByteArray bytes; -}; + return test != '_' && (test < 'a' || test > 'z' ) && (test < 'A' || test > 'Z'); +} -struct ByteArrayHasher +/// Get the integer suffix of name. Returns a tuple of (suffix, suffixPosition). Calling code +/// should check to ensure that suffixPosition is not equal to nameLength (in which case there was no +/// suffix). +/// +/// \param name The name to check +/// \param nameLength The length of the string in name +/// \returns An integer pair of the suffix itself and the position of that suffix in name +std::pair getIntegerSuffix(const char *name, int nameLength) { - std::size_t operator()(const ByteArray& bytes) const - { - return qHash(bytes.bytes); - } + int suffixPosition {nameLength - 1}; - std::size_t operator()(const QByteArray& bytes) const - { - return qHash(bytes); + for (; suffixPosition >= 0; --suffixPosition) { + // When we support C++20 we can use std::span<> to eliminate the clang-tidy warning + // NOLINTNEXTLINE cppcoreguidelines-pro-bounds-pointer-arithmetic + if (!isdigit(name[suffixPosition])) { + break; + } } -}; + ++suffixPosition; + int suffix {0}; + if (suffixPosition < nameLength) { + // When we support C++20 we can use std::span<> to eliminate the clang-tidy warning + // NOLINTNEXTLINE cppcoreguidelines-pro-bounds-pointer-arithmetic + suffix = std::atoi(name + suffixPosition); + } + return std::make_pair(suffix, suffixPosition); +} void IndexedName::set( const char* name, - int len, - const std::vector& types, + int length, + const std::vector& allowedNames, bool allowOthers) { + // Storage for names that we weren't given external storage for static std::unordered_set NameSet; - if (len < 0) - len = static_cast(std::strlen(name)); - int i; - for (i = len - 1; i >= 0; --i) { - if (name[i] < '0' || name[i]>'9') - break; + if (length < 0) { + length = static_cast(std::strlen(name)); + } + // Name typically ends with an integer: find that integer + auto [suffix, suffixPosition] = getIntegerSuffix(name, length); + if (suffixPosition < length) { + this->index = suffix; } - ++i; - this->index = std::atoi(name + i); - for (int j = 0; j < i; ++j) { - if (name[j] == '_' - || (name[j] >= 'a' && name[j] <= 'z') - || (name[j] >= 'A' && name[j] <= 'Z')) - continue; + // Make sure that every character is either an ASCII letter (upper or lowercase), or an + // underscore. If any other character appears, reject the entire string. + // When we support C++20 we can use std::span<> to eliminate the clang-tidy warning + // NOLINTNEXTLINE cppcoreguidelines-pro-bounds-pointer-arithmetic + if (std::any_of(name, name+suffixPosition, isInvalidChar)) { this->type = ""; return; } - for (const char* type : types) { - int j = 0; - for (const char* n = name, *t = type; *n; ++n) { - if (*n != *t || j >= i) - break; - ++j; - ++t; - if (!*t) { - this->type = type; - return; - } + // If a list of allowedNames was provided, see if our set name matches one of those allowedNames: if it + // does, reference that memory location and return. + for (const auto *typeName : allowedNames) { + if (std::strncmp(name, typeName, suffixPosition) == 0) { + this->type = typeName; + return; } } + // If the type was NOT in the list of allowedNames, but the caller has set the allowOthers flag to + // true, then add the new type to the static NameSet (if it is not already there). if (allowOthers) { - auto res = NameSet.insert(QByteArray::fromRawData(name, i)); - if (res.second) - res.first->mutate(); + auto res = NameSet.insert(ByteArray(QByteArray::fromRawData(name, suffixPosition))); + if (res.second /*The insert succeeded (the type was new)*/) { + // Make sure that the data in the set is a unique (unshared) copy of the text + res.first->ensureUnshared(); + } this->type = res.first->bytes.constData(); } - else + else { + // The passed-in type is not in the allowed list, and allowOthers was not true, so don't + // store the type this->type = ""; + } } diff --git a/src/App/IndexedName.h b/src/App/IndexedName.h index 3e6488293492..85282b9545c4 100644 --- a/src/App/IndexedName.h +++ b/src/App/IndexedName.h @@ -2,6 +2,7 @@ /**************************************************************************** * Copyright (c) 2022 Zheng, Lei (realthunder) * + * Copyright (c) 2023 FreeCAD Project Association * * * * This file is part of FreeCAD. * * * @@ -21,9 +22,8 @@ * * ***************************************************************************/ - -#ifndef _IndexedName_h_ -#define _IndexedName_h_ +#ifndef APP_INDEXEDNAME_H +#define APP_INDEXEDNAME_H #include #include @@ -32,6 +32,7 @@ #include #include +#include #include "FCGlobal.h" @@ -39,65 +40,133 @@ namespace Data { +/// The IndexedName class provides a very memory-efficient data structure to hold a name and an index +/// value, and to perform various comparisons and validations of those values. The name must only +/// consist of upper- and lower-case ASCII characters and the underscore ('_') character. The index +/// must be a positive integer. The string representation of this IndexedName is the name followed by +/// the index, with no spaces between: an IndexedName may be constructed from this string. For +/// example "EDGE1" or "FACE345" might be the names of elements that use an IndexedName. If there is +/// then an "EDGE2", only a pointer to the original stored name "EDGE" is retained. +/// +/// The memory efficiency of the class comes from re-using the same character storage for names that +/// match, while retaining their differing indices. This is achieved by either using user-provided +/// const char * names (provided as a list of typeNames and presumed to never be deallocated), or by +/// maintaining an internal list of names that have been used before, and can be re-used later. class AppExport IndexedName { public: + + /// Construct from a name and an optional index. If the name contains an index it is read, but + /// is used as the index *only* if _index parameter is unset. If the _index parameter is given + /// it overrides any trailing integer in the name. Index must be positive, and name must contain + /// only ASCII letters and the underscore character. If these conditions are not met, name is + /// set to the empty string, and isNull() will return true. + /// + /// \param name The new name - ASCII letters and underscores only, with optional integer suffix. + /// This memory will be copied into a new internal storage location and need not be persistent. + /// \param _index The new index - if provided, it overrides any suffix provided by name explicit IndexedName(const char *name = nullptr, int _index = 0) : index(0) { - if (!name) + assert(_index >= 0); + if (!name) { this->type = ""; + } else { set(name); - if (_index) + if (_index > 0) { this->index = _index; + } } } + /// Create an indexed name that is restricted to a list of preset type names. If it appears in + /// that list, only a pointer to the character storage in the list is retained: the memory + /// locations pointed at by the list must never be destroyed once they have been used to create + /// names. If allowOthers is true (the default) then a requested name that is not in the list + /// will be added to a static internal storage table, and its memory then re-used for later + /// objects with the same name. If allowOthers is false, then the name request is rejected, and + /// the name is treated as null. + /// + /// \param name The new name - ASCII letters and underscores only, with optional integer suffix + /// \param allowedTypeNames A vector of allowed names. Storage locations must persist for the + /// entire run of the program. + /// \param allowOthers Whether a name not in allowedTypeNames is permitted. If true (the + /// default) then a name not in allowedTypeNames is added to a static internal storage vector + /// so that it can be re-used later without additional memory allocation. IndexedName(const char *name, - const std::vector & types, - bool allowOthers=true) + const std::vector & allowedTypeNames, + bool allowOthers=true) : type(""), index(0) { - set(name, -1, types, allowOthers); + set(name, -1, allowedTypeNames, allowOthers); } - explicit IndexedName(const QByteArray & data) + /// Construct from a QByteArray, but explicitly making a copy of the name on its first + /// occurrence. If this is a name that has already been stored internally, no additional copy + /// is made. + /// + /// \param data The QByteArray to copy the data from + explicit IndexedName(const QByteArray & data) : type(""), index(0) { set(data.constData(), data.size()); } - IndexedName(const IndexedName &other) - : type(other.type), index(other.index) - {} - + /// Given constant name and an index, re-use the existing memory for the name, not making a copy + /// of it, or scanning any existing storage for it. The name must never become invalid for the + /// lifetime of the object it names. This memory will never be re-used by another object. + /// + /// \param name The name of the object. This memory is NOT copied and must be persistent. + /// \param index A positive, non-zero integer + /// \return An IndexedName with the given name and index, re-using the existing memory for name static IndexedName fromConst(const char *name, int index) { + assert (index >= 0); IndexedName res; res.type = name; res.index = index; return res; } - IndexedName & operator=(const IndexedName & other) + /// Given an existing std::string, *append* this name to it. If index is not zero, this will + /// include the index. + /// + /// \param buffer A (possibly non-empty) string buffer to append the name to. + void appendToStringBuffer(std::string & buffer) const { - this->index = other.index; - this->type = other.type; - return *this; + buffer += this->type; + if (this->index > 0) { + buffer += std::to_string(this->index); + } } - friend std::ostream & operator<<(std::ostream & s, const IndexedName & e) + /// Create and return a new std::string with this name in it. + /// + /// \return A newly-created string with the IndexedName in it (e.g. "EDGE42") + std::string toString() const { - s << e.type; - if (e.index > 0) - s << e.index; - return s; + std::string result; + this->appendToStringBuffer(result); + return result; } + /// An indexedName is represented as the simple concatenation of the name and its index, e.g. + /// "EDGE1" or "FACE42". + friend std::ostream & operator<<(std::ostream & stream, const IndexedName & indexedName) + { + stream << indexedName.type; + if (indexedName.index > 0) { + stream << indexedName.index; + } + return stream; + } + + /// True only if both the name and index compare exactly equal. bool operator==(const IndexedName & other) const { return this->index == other.index && (this->type == other.type - || std::strcmp(this->type, other.type)==0); + || std::strcmp(this->type, other.type)==0); } + /// Increments the index by the given offset. Does not affect the text part of the name. IndexedName & operator+=(int offset) { this->index += offset; @@ -105,12 +174,15 @@ class AppExport IndexedName { return *this; } + /// Pre-increment operator: increases the index of this element by one. IndexedName & operator++() { ++this->index; return *this; } + /// Pre-decrement operator: decreases the index of this element by one. Must not make the index + /// negative (only checked when compiled in debug mode). IndexedName & operator--() { --this->index; @@ -118,57 +190,83 @@ class AppExport IndexedName { return *this; } + /// True if either the name or the index compare not equal. bool operator!=(const IndexedName & other) const { return !(this->operator==(other)); } - const char * toString(std::string & s) const - { - // Note! s is not cleared on purpose. - std::size_t offset = s.size(); - s += this->type; - if (this->index > 0) - s += std::to_string(this->index); - return s.c_str() + offset; - } - + /// Equivalent to C++20's operator <=> int compare(const IndexedName & other) const { - int res = std::strcmp(this->type, other.type); - if (res) + int res = std::strcmp(this->type, other.type); + if (res != 0) { return res; - if (this->index < other.index) + } + if (this->index < other.index) { return -1; - if (this->index > other.index) + } + if (this->index > other.index) { return 1; + } return 0; } + /// Provided to enable sorting operations: the comparison is first lexicographical for the text + /// element of the names, then numerical for the indices. bool operator<(const IndexedName & other) const { return compare(other) < 0; } - char operator[](int index) const - { - return this->type[index]; - } + /// Allow direct memory access to the individual characters of the text portion of the name. + /// NOTE: input is not range-checked when compiled in release mode. + char operator[](int input) const + { + assert(input >= 0); + assert(input < static_cast(std::strlen(this->type))); + // When we support C++20 we can use std::span<> to eliminate the clang-tidy warning + // NOLINTNEXTLINE cppcoreguidelines-pro-bounds-pointer-arithmetic + return this->type[input]; + } - const char * getType() const { return this->type; } + /// Get a pointer to text part of the name - does NOT make a copy, returns direct memory access + const char * getType() const { return this->type; } - int getIndex() const { return this->index; } + /// Get the numerical part of the name + int getIndex() const { return this->index; } - void setIndex(int index) { assert(index>=0); this->index = index; } + /// Set the numerical part of the name (note that there is no equivalent function to allow + /// changing the text part of the name, which is immutable once created). + /// + /// \param input The new index. Must be a positive non-zero integer + void setIndex(int input) { assert(input>=0); this->index = input; } - bool isNull() const { return !this->type[0]; } + /// A name is considered "null" if its text component is an empty string. + // When we support C++20 we can use std::span<> to eliminate the clang-tidy warning + // NOLINTNEXTLINE cppcoreguidelines-pro-bounds-pointer-arithmetic + bool isNull() const { return this->type[0] == '\0'; } + /// Boolean conversion provides the opposite of isNull(), yielding true when the text part of + /// the name is NOT the empty string. explicit operator bool() const { return !isNull(); } protected: - void set(const char *, - int len = -1, - const std::vector &types = {}, + /// Apply the IndexedName rules and either store the characters of a new type or a reference to + /// the characters in a type named in types, or stored statically within this function. If len + /// is not set, or set to -1 (the default), then the provided string in name is scanned for its + /// length using strlen (e.g. it must be null-terminated). + /// + /// \param name The new name. If necessary a copy is made, this char * need not be persistent + /// \param length The length of name + /// \param allowedNames A vector of storage locations of allowed names. These storage locations + /// must be persistent for the duration of the program run. + /// \param allowOthers If true (the default), then if name is not in allowedNames it is allowed, + /// and it is added to internal storage (making a copy of the name if this is its first + /// occurrence). + void set(const char *name, + int length = -1, + const std::vector & allowedNames = {}, bool allowOthers = true); private: @@ -176,6 +274,65 @@ class AppExport IndexedName { int index; }; + +/// A thin wrapper around a QByteArray providing the ability to force a copy of the data at any +/// time, even if it isn't being written to. The standard assignment operator for this class *does* +/// make a copy of the data, unlike the standard assignment operator for QByteArray. +struct ByteArray +{ + explicit ByteArray(QByteArray other) + :bytes(std::move(other)) + {} + + ByteArray(const ByteArray& other) = default; + + ByteArray(ByteArray&& other) noexcept + :bytes(std::move(other.bytes)) + {} + + ~ByteArray() = default; + + /// Guarantee that the stored QByteArray does not share its memory with another instance. + void ensureUnshared() const + { + QByteArray copy; + copy.append(bytes.constData(), bytes.size()); + bytes = copy; + } + + bool operator==(const ByteArray& other) const { + return bytes == other.bytes; + } + + ByteArray &operator=(const ByteArray & other) { + bytes.clear(); + bytes.append(other.bytes.constData(), other.bytes.size()); + return *this; + } + + ByteArray &operator= (ByteArray&& other) noexcept + { + bytes = std::move(other.bytes); + return *this; + } + + mutable QByteArray bytes; +}; + + +struct ByteArrayHasher +{ + std::size_t operator()(const ByteArray& bytes) const + { + return qHash(bytes.bytes); + } + + std::size_t operator()(const QByteArray& bytes) const + { + return qHash(bytes); + } +}; + } -#endif _IndexedName_h_ +#endif // APP_INDEXEDNAME_H diff --git a/tests/src/App/CMakeLists.txt b/tests/src/App/CMakeLists.txt index 0b4eaed5ebf7..fbde1bb2fa28 100644 --- a/tests/src/App/CMakeLists.txt +++ b/tests/src/App/CMakeLists.txt @@ -3,6 +3,7 @@ target_sources( PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/Branding.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Expression.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/IndexedName.cpp ${CMAKE_CURRENT_SOURCE_DIR}/License.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Metadata.cpp ) diff --git a/tests/src/App/IndexedName.cpp b/tests/src/App/IndexedName.cpp new file mode 100644 index 000000000000..914fb4f4332b --- /dev/null +++ b/tests/src/App/IndexedName.cpp @@ -0,0 +1,595 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "gtest/gtest.h" + +#include "App/IndexedName.h" + +#include + +// NOLINTBEGIN(readability-magic-numbers) + +class IndexedNameTest : public ::testing::Test { +protected: + // void SetUp() override {} + + // void TearDown() override {} + + // Create and return a list of invalid IndexedNames + static std::vector givenInvalidIndexedNames() { + return std::vector { + Data::IndexedName(), + Data::IndexedName("",1), + Data::IndexedName("INVALID42NAME",1), + Data::IndexedName(".EDGE",1) + }; + } + + // Create and return a list of valid IndexedNames + static std::vector givenValidIndexedNames() { + return std::vector { + Data::IndexedName("NAME"), + Data::IndexedName("NAME1"), + Data::IndexedName("NAME",1), + Data::IndexedName("NAME_WITH_UNDERSCORES12345") + }; + } + + // An arbitrary list of C strings used for testing some types of construction + // NOLINTNEXTLINE cppcoreguidelines-non-private-member-variables-in-classes + std::vector allowedTypes { + "VERTEX", + "EDGE", + "FACE", + "WIRE" + }; +}; + +TEST_F(IndexedNameTest, defaultConstruction) +{ + // Act + auto indexedName = Data::IndexedName(); + + // Assert + EXPECT_STREQ(indexedName.getType(), ""); + EXPECT_EQ(indexedName.getIndex(), 0); +} + +TEST_F(IndexedNameTest, nameOnlyConstruction) +{ + // Act + auto indexedName = Data::IndexedName("TestName"); + + // Assert + EXPECT_STREQ(indexedName.getType(), "TestName"); + EXPECT_EQ(indexedName.getIndex(), 0); +} + +TEST_F(IndexedNameTest, nameAndIndexConstruction) +{ + // Arrange + const int testIndex {42}; + + // Act + auto indexedName = Data::IndexedName("TestName", testIndex); + + // Assert + EXPECT_STREQ(indexedName.getType(), "TestName"); + EXPECT_EQ(indexedName.getIndex(), testIndex); +} + +TEST_F(IndexedNameTest, nameAndIndexConstructionWithOverride) +{ + // Arrange + const int testIndex {42}; + + // Act + auto indexedName = Data::IndexedName("TestName17", testIndex); + + // Assert + EXPECT_STREQ(indexedName.getType(), "TestName"); + EXPECT_EQ(indexedName.getIndex(), testIndex); +} + +// Names must only contain ASCII letters and underscores (but may end with a number) +TEST_F(IndexedNameTest, constructionInvalidCharInName) +{ + // Arrange + constexpr int lastASCIICode{127}; + std::vector illegalCharacters = {}; + for (int code = 1; code <= lastASCIICode; ++code) { + if ((std::isalnum(code) == 0) && code != '_') { + illegalCharacters.push_back(char(code)); + } + } + for (auto illegalChar : illegalCharacters) { + std::string testName {"TestName"}; + testName += illegalChar; + + // Act + auto indexedName = Data::IndexedName(testName.c_str(), 1); + + // Assert + EXPECT_STREQ(indexedName.getType(), "") << "Expected empty name when given " << testName; + } +} + +// Names must not contain numbers in the middle: +TEST_F(IndexedNameTest, constructionNumberInName) +{ + // Arrange + const int testIndex {42}; + std::string testName; + testName += "Test" + std::to_string(testIndex) + "Name"; + + // Act + auto indexedName = Data::IndexedName(testName.c_str(), testIndex); + + // Assert + EXPECT_STREQ(indexedName.getType(), ""); +} + +TEST_F(IndexedNameTest, nameAndTypeListConstructionWithoutAllowOthers) +{ + // Act + auto indexedName = Data::IndexedName("EDGE19", allowedTypes, false); + + // Assert + EXPECT_STREQ(indexedName.getType(), "EDGE"); + EXPECT_EQ(indexedName.getIndex(), 19); + + // Act + indexedName = Data::IndexedName("EDGES_ARE_REALLY_GREAT19", allowedTypes, false); + + // Assert + EXPECT_STREQ(indexedName.getType(), ""); + EXPECT_EQ(indexedName.getIndex(), 19); + + // Act + indexedName = Data::IndexedName("NOT_IN_THE_LIST42", allowedTypes, false); + + // Assert + EXPECT_STREQ(indexedName.getType(), ""); +} + +TEST_F(IndexedNameTest, nameAndTypeListConstructionWithAllowOthers) +{ + // Act + auto indexedName = Data::IndexedName("NOT_IN_THE_LIST42", allowedTypes, true); + + // Assert + EXPECT_STREQ(indexedName.getType(), "NOT_IN_THE_LIST"); + EXPECT_EQ(indexedName.getIndex(), 42); +} + +// Check that the same memory location is used for two names that are not in the allowedTypes list +TEST_F(IndexedNameTest, nameAndTypeListConstructionReusedMemoryCheck) +{ + // Arrange + auto indexedName1 = Data::IndexedName("NOT_IN_THE_LIST42", allowedTypes, true); + auto indexedName2 = Data::IndexedName("NOT_IN_THE_LIST43", allowedTypes, true); + + // Act & Assert + EXPECT_EQ(indexedName1.getType(), indexedName2.getType()); +} + +TEST_F(IndexedNameTest, byteArrayConstruction) +{ + // Arrange + QByteArray qba{"EDGE42"}; + + // Act + auto indexedName = Data::IndexedName(qba); + + // Assert + EXPECT_STREQ(indexedName.getType(), "EDGE"); + EXPECT_EQ(indexedName.getIndex(), 42); +} + +TEST_F(IndexedNameTest, copyConstruction) +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE42"); + + // Act + auto indexedNameCopy {indexedName}; + + // Assert + EXPECT_EQ(indexedName, indexedNameCopy); +} + +TEST_F(IndexedNameTest, streamInsertionOperator) +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE42"); + std::stringstream ss; + + // Act + ss << indexedName; + + // Assert + EXPECT_EQ(ss.str(), std::string{"EDGE42"}); +} + +TEST_F(IndexedNameTest, compoundAssignmentOperator) +{ + // NOTE: Only += is defined for this class + + // Arrange + constexpr int base{42}; + constexpr int offset{10}; + auto indexedName = Data::IndexedName("EDGE",base); + + // Act + indexedName += offset; + + // Assert + EXPECT_EQ(indexedName.getIndex(), 52); +} + +TEST_F(IndexedNameTest, preincrementOperator) +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE42"); + + // Act + ++indexedName; + + // Assert + EXPECT_EQ(indexedName.getIndex(), 43); +} + +TEST_F(IndexedNameTest, predecrementOperator) +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE42"); + + // Act + --indexedName; + + // Assert + EXPECT_EQ(indexedName.getIndex(), 41); +} + +TEST_F(IndexedNameTest, comparisonOperators) +{ + // Arrange + auto indexedName1 = Data::IndexedName("EDGE42"); + auto indexedName2 = Data::IndexedName("EDGE42"); + + // Act & Assert + EXPECT_EQ(indexedName1.compare(indexedName2), 0); + EXPECT_TRUE(indexedName1 == indexedName2); + EXPECT_FALSE(indexedName1 != indexedName2); + EXPECT_FALSE(indexedName1 < indexedName2); + + // Arrange + auto indexedName3 = Data::IndexedName("EDGE42"); + auto indexedName4 = Data::IndexedName("FACE42"); + + // Act & Assert + EXPECT_EQ(indexedName3.compare(indexedName4), -1); + EXPECT_FALSE(indexedName3 == indexedName4); + EXPECT_TRUE(indexedName3 != indexedName4); + EXPECT_TRUE(indexedName3 < indexedName4); + + // Arrange + auto indexedName5 = Data::IndexedName("FACE42"); + auto indexedName6 = Data::IndexedName("EDGE42"); + + // Act & Assert + EXPECT_EQ(indexedName5.compare(indexedName6), 1); + EXPECT_FALSE(indexedName5 == indexedName6); + EXPECT_TRUE(indexedName5 != indexedName6); + EXPECT_FALSE(indexedName5 < indexedName6); + + // Arrange + auto indexedName7 = Data::IndexedName("EDGE41"); + auto indexedName8 = Data::IndexedName("EDGE42"); + + // Act & Assert + EXPECT_EQ(indexedName7.compare(indexedName8), -1); + EXPECT_FALSE(indexedName7 == indexedName8); + EXPECT_TRUE(indexedName7 != indexedName8); + EXPECT_TRUE(indexedName7 < indexedName8); + + // Arrange + auto indexedName9 = Data::IndexedName("EDGE43"); + auto indexedName10 = Data::IndexedName("EDGE42"); + + // Act & Assert + EXPECT_EQ(indexedName9.compare(indexedName10), 1); + EXPECT_FALSE(indexedName9 == indexedName10); + EXPECT_TRUE(indexedName9 != indexedName10); + EXPECT_FALSE(indexedName9 < indexedName10); +} + +TEST_F(IndexedNameTest, subscriptOperator) +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE42"); + + // Act & Assert + EXPECT_EQ(indexedName[0], 'E'); + EXPECT_EQ(indexedName[1], 'D'); + EXPECT_EQ(indexedName[2], 'G'); + EXPECT_EQ(indexedName[3], 'E'); +} + +TEST_F(IndexedNameTest, getType) +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE42"); + + // Act & Assert + EXPECT_STREQ(indexedName.getType(), "EDGE"); +} + +TEST_F(IndexedNameTest, setIndex) +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE42"); + EXPECT_EQ(indexedName.getIndex(), 42); + + // Act + indexedName.setIndex(1); + + // Assert + EXPECT_EQ(indexedName.getIndex(), 1); +} + +TEST_F(IndexedNameTest, isNullTrue) +{ + // Arrange + auto invalidNames = givenInvalidIndexedNames(); + for (const auto &name : invalidNames) { + + // Act & Assert + EXPECT_TRUE(name.isNull()); + } +} + +TEST_F(IndexedNameTest, isNullFalse) +{ + // Arrange + auto validNames = givenValidIndexedNames(); + for (const auto &name : validNames) { + + // Act & Assert + EXPECT_FALSE(name.isNull()); + } +} + +TEST_F(IndexedNameTest, booleanConversionFalse) +{ + // Arrange + auto invalidNames = givenInvalidIndexedNames(); + for (const auto &name : invalidNames) { + + // Act & Assert + EXPECT_FALSE(static_cast(name)); + } + + // Usage example: + auto indexedName = Data::IndexedName(".EDGE",1); // Invalid name + if (indexedName) { + FAIL() << "indexedName as a boolean should have been false for an invalid name"; + } +} + +TEST_F(IndexedNameTest, booleanConversionTrue) +{ + // Arrange + auto validNames = givenValidIndexedNames(); + for (const auto& name : validNames) { + + // Act & Assert + EXPECT_TRUE(static_cast(name)); + } +} + +TEST_F(IndexedNameTest, fromConst) +{ + // Arrange + const int testIndex {42}; + + // Act + auto indexedName = Data::IndexedName::fromConst("TestName", testIndex); + + // Assert + EXPECT_STREQ(indexedName.getType(), "TestName"); + EXPECT_EQ(indexedName.getIndex(), testIndex); +} + +TEST_F(IndexedNameTest, appendToStringBufferEmptyBuffer) +{ + // Arrange + std::string bufferStartedEmpty; + Data::IndexedName testName("TEST_NAME", 1); + + // Act + testName.appendToStringBuffer(bufferStartedEmpty); + + // Assert + EXPECT_EQ(bufferStartedEmpty, "TEST_NAME1"); +} + +TEST_F(IndexedNameTest, appendToStringBufferNonEmptyBuffer) +{ + // Arrange + std::string bufferWithData {"DATA"}; + Data::IndexedName testName("TEST_NAME", 1); + + // Act + testName.appendToStringBuffer(bufferWithData); + + // Assert + EXPECT_EQ(bufferWithData, "DATATEST_NAME1"); +} + +TEST_F(IndexedNameTest, appendToStringBufferZeroIndex) +{ + // Arrange + std::string bufferStartedEmpty; + Data::IndexedName testName("TEST_NAME", 0); + + // Act + testName.appendToStringBuffer(bufferStartedEmpty); + + // Assert + EXPECT_EQ(bufferStartedEmpty, "TEST_NAME"); +} + +TEST_F(IndexedNameTest, toString) +{ + // Arrange + Data::IndexedName testName("TEST_NAME", 1); + + // Act + auto result = testName.toString(); + + // Assert + EXPECT_EQ(result, "TEST_NAME1"); +} + +TEST_F(IndexedNameTest, toStringNoIndex) +{ + // Arrange + Data::IndexedName testName("TEST_NAME", 0); + + // Act + auto result = testName.toString(); + + // Assert + EXPECT_EQ(result, "TEST_NAME"); +} + +TEST_F(IndexedNameTest, assignmentOperator) +{ + // Arrange + const int testIndex1 {42}; + const int testIndex2 {24}; + auto indexedName1 = Data::IndexedName::fromConst("TestName", testIndex1); + auto indexedName2 = Data::IndexedName::fromConst("TestName2", testIndex2); + EXPECT_NE(indexedName1, indexedName2); // Ensure the test is set up correctly + + // Act + indexedName1 = indexedName2; + + // Assert + EXPECT_EQ(indexedName1, indexedName2); +} + + +class ByteArrayTest : public ::testing::Test { +protected: + // void SetUp() override {} + + // void TearDown() override {} +}; + +TEST_F(ByteArrayTest, QByteArrayConstruction) +{ + // Arrange + QByteArray testQBA("Data in a QByteArray"); + + // Act + Data::ByteArray testByteArray (testQBA); + + // Assert + EXPECT_EQ(testQBA, testByteArray.bytes); +} + +TEST_F(ByteArrayTest, CopyConstruction) +{ + // Arrange + QByteArray testQBA("Data in a QByteArray"); + Data::ByteArray originalByteArray (testQBA); + + // Act + // NOLINTNEXTLINE performance-unnecessary-copy-initialization + Data::ByteArray copiedByteArray (originalByteArray); + + // Assert + EXPECT_EQ(originalByteArray, copiedByteArray); +} + +TEST_F(ByteArrayTest, MoveConstruction) +{ + // Arrange + QByteArray testQBA("Data in a QByteArray"); + Data::ByteArray originalByteArray (testQBA); + const auto *originalDataLocation = originalByteArray.bytes.constData(); + + // Act + Data::ByteArray copiedByteArray (std::move(originalByteArray)); + + // Assert + EXPECT_EQ(testQBA, copiedByteArray.bytes); + EXPECT_EQ(originalDataLocation, copiedByteArray.bytes.constData()); +} + +TEST_F(ByteArrayTest, ensureUnshared) +{ + // Arrange + QByteArray testQBA("Data in a QByteArray"); + Data::ByteArray originalByteArray (testQBA); + const auto *originalDataLocation = originalByteArray.bytes.constData(); + Data::ByteArray copiedByteArray (originalByteArray); + + // Act + copiedByteArray.ensureUnshared(); + + // Assert + EXPECT_EQ(testQBA, copiedByteArray.bytes); + EXPECT_NE(originalDataLocation, copiedByteArray.bytes.constData()); +} + +TEST_F(ByteArrayTest, equalityOperator) +{ + // Arrange + QByteArray testQBA1("Data in a QByteArray"); + QByteArray testQBA2("Data in a QByteArray"); + QByteArray testQBA3("Not the same data in a QByteArray"); + Data::ByteArray byteArray1 (testQBA1); + Data::ByteArray byteArray2 (testQBA2); + Data::ByteArray byteArray3 (testQBA3); + + // Act & Assert + EXPECT_TRUE(byteArray1 == byteArray2); + EXPECT_FALSE(byteArray1 == byteArray3); +} + +TEST_F(ByteArrayTest, assignmentOperator) +{ + // Arrange + QByteArray testQBA1("Data in a QByteArray"); + QByteArray testQBA2("Different data in a QByteArray"); + Data::ByteArray originalByteArray (testQBA1); + Data::ByteArray newByteArray (testQBA2); + ASSERT_FALSE(originalByteArray == newByteArray); + + // Act + newByteArray = originalByteArray; + + // Assert + EXPECT_TRUE(originalByteArray == newByteArray); +} + +TEST_F(ByteArrayTest, moveAssignmentOperator) +{ + // Arrange + QByteArray testQBA1("Data in a QByteArray"); + QByteArray testQBA2("Different data in a QByteArray"); + Data::ByteArray originalByteArray (testQBA1); + const auto *originalByteArrayLocation = originalByteArray.bytes.constData(); + Data::ByteArray newByteArray (testQBA2); + ASSERT_FALSE(originalByteArray == newByteArray); + + // Act + newByteArray = std::move(originalByteArray); + + // Assert + EXPECT_EQ(originalByteArrayLocation, newByteArray.bytes.constData()); +} + +// NOLINTEND(readability-magic-numbers)