From e7f8f26bd57f076ac955a81ea8a6c839a6eb3b50 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Thu, 6 Jul 2023 10:12:55 -0500 Subject: [PATCH] App/Toponaming: Basic tests and linter issues --- src/App/ComplexGeoData.cpp | 187 +++---- src/App/ComplexGeoData.h | 68 +-- src/App/ElementMap.cpp | 875 ++++++++++++++++++------------- src/App/ElementMap.h | 34 +- src/App/MappedName.h | 96 ++-- src/App/StringHasher.h | 3 +- tests/src/App/CMakeLists.txt | 1 + tests/src/App/ComplexGeoData.cpp | 417 +++++++++++++++ tests/src/App/ElementMap.cpp | 20 +- tests/src/App/MappedName.cpp | 17 +- 10 files changed, 1138 insertions(+), 580 deletions(-) create mode 100644 tests/src/App/ComplexGeoData.cpp diff --git a/src/App/ComplexGeoData.cpp b/src/App/ComplexGeoData.cpp index fe5d07ece6..5c6fd3291a 100644 --- a/src/App/ComplexGeoData.cpp +++ b/src/App/ComplexGeoData.cpp @@ -24,7 +24,7 @@ ***************************************************************************/ -#include "PreCompiled.h" +#include "PreCompiled.h"// NOLINT #ifndef _PreComp_ # include @@ -41,21 +41,19 @@ #include #include -#include using namespace Data; -TYPESYSTEM_SOURCE_ABSTRACT(Data::Segment , Base::BaseClass) +TYPESYSTEM_SOURCE_ABSTRACT(Data::Segment , Base::BaseClass)// NOLINT +TYPESYSTEM_SOURCE_ABSTRACT(Data::ComplexGeoData , Base::Persistence)// NOLINT - -TYPESYSTEM_SOURCE_ABSTRACT(Data::ComplexGeoData , Base::Persistence) - -FC_LOG_LEVEL_INIT("ComplexGeoData", true,true) +FC_LOG_LEVEL_INIT("ComplexGeoData", true,true)// NOLINT namespace bio = boost::iostreams; using namespace Data; +// NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) ComplexGeoData::ComplexGeoData() :Tag(0) @@ -107,10 +105,10 @@ Base::Placement ComplexGeoData::getPlacement() const { Base::Matrix4D mat = getTransform(); - return Base::Placement(Base::Vector3d(mat[0][3], - mat[1][3], - mat[2][3]), - Base::Rotation(mat)); + return {Base::Vector3d(mat[0][3], + mat[1][3], + mat[2][3]), + Base::Rotation(mat)}; } double ComplexGeoData::getAccuracy() const @@ -118,19 +116,21 @@ double ComplexGeoData::getAccuracy() const return 0.0; } -void ComplexGeoData::getLinesFromSubElement(const Segment*, +void ComplexGeoData::getLinesFromSubElement(const Segment* segment, std::vector &Points, std::vector &lines) const { + (void)segment; (void)Points; (void)lines; } -void ComplexGeoData::getFacesFromSubElement(const Segment*, +void ComplexGeoData::getFacesFromSubElement(const Segment* segment, std::vector &Points, std::vector &PointNormals, std::vector &faces) const { + (void)segment; (void)Points; (void)PointNormals; (void)faces; @@ -174,8 +174,9 @@ void ComplexGeoData::getFaces(std::vector &Points, (void)flags; } -bool ComplexGeoData::getCenterOfGravity(Base::Vector3d&) const +bool ComplexGeoData::getCenterOfGravity(Base::Vector3d& unused) const { + (void)unused; return false; } @@ -195,18 +196,21 @@ MappedName ComplexGeoData::getMappedName(const IndexedName & element, bool allowUnmapped, ElementIDRefs *sid) const { - if (!element) - return MappedName(); + if (!element) { + return {}; + } flushElementMap(); if(!_elementMap) { - if (allowUnmapped) + if (allowUnmapped) { return MappedName(element); - return MappedName(); + } + return {}; } MappedName name = _elementMap->find(element, sid); - if (allowUnmapped && !name) + if (allowUnmapped && !name) { return MappedName(element); + } return name; } @@ -214,11 +218,12 @@ IndexedName ComplexGeoData::getIndexedName(const MappedName & name, ElementIDRefs *sid) const { flushElementMap(); - if (!name) + if (!name) { return IndexedName(); + } if (!_elementMap) { - std::string s; - return IndexedName(name.appendToBuffer(s), getElementTypes()); + std::string str; + return {name.appendToBuffer(str), getElementTypes()}; } return _elementMap->find(name, sid); } @@ -229,24 +234,29 @@ ComplexGeoData::getElementName(const char *name, bool copy) const { IndexedName element(name, getElementTypes()); - if (element) - return MappedElement(getMappedName(element, false, sid), element); + if (element) { + return {getMappedName(element, false, sid), element}; + } const char * mapped = isMappedElement(name); - if (mapped) + if (mapped) { name = mapped; + } - MappedElement res; + MappedElement result; // Strip out the trailing '.XXXX' if any const char *dot = strchr(name,'.'); - if(dot) - res.name = MappedName(name, dot-name); - else if (copy) - res.name = name; - else - res.name = MappedName(name); - res.index = getIndexedName(res.name, sid); - return res; + if(dot) { + result.name = MappedName(name, dot - name); + } + else if (copy) { + result.name = name; + } + else { + result.name = MappedName(name); + } + result.index = getIndexedName(result.name, sid); + return result; } std::vector > @@ -254,44 +264,30 @@ ComplexGeoData::getElementMappedNames(const IndexedName & element, bool needUnma flushElementMap(); if(_elementMap) { auto res = _elementMap->findAll(element); - if (!res.empty()) + if (!res.empty()) { return res; + } } - if (!needUnmapped) + if (!needUnmapped) { return {}; + } return {std::make_pair(MappedName(element), ElementIDRefs())}; } -std::vector -ComplexGeoData::getElementNamesWithPrefix(const char *prefix) const { -#if 0 - std::vector names; - flushElementMap(); - if(!prefix || !prefix[0] || !_elementMap) - return names; - const auto &p = elementMapPrefix(); - if(boost::starts_with(prefix,p)) - prefix += p.size(); - names = _elementMap->findAllStartsWith(prefix); - return names; -#else - (void)prefix; - return {}; -#endif -} - std::vector ComplexGeoData::getElementMap() const { flushElementMap(); - if(!_elementMap) + if(!_elementMap) { return {}; + } return _elementMap->getAll(); } ElementMapPtr ComplexGeoData::elementMap(bool flush) const { - if (flush) + if (flush) { flushElementMap(); + } return _elementMap; } @@ -300,82 +296,93 @@ void ComplexGeoData::flushElementMap() const } void ComplexGeoData::setElementMap(const std::vector &map) { - resetElementMap(); - for(auto &v : map) - _elementMap->setElementName(v.index, v.name, Tag); + _elementMap = std::make_shared(); // Get rid of the old one, if any, but make + // sure the memory exists for the new data. + for(auto &element : map) { + _elementMap->setElementName(element.index, element.name, Tag); + } } char ComplexGeoData::elementType(const Data::MappedName &name) const { - if(!name) + if(!name) { return 0; + } auto indexedName = getIndexedName(name); - if (indexedName) + if (indexedName) { return elementType(indexedName); + } char element_type=0; - if (name.findTagInElementName(0,0,0,&element_type) < 0) + if (name.findTagInElementName(nullptr,nullptr,nullptr,&element_type) < 0) { return elementType(name.toIndexedName()); + } return element_type; } char ComplexGeoData::elementType(const Data::IndexedName &element) const { - if(!element) + if(!element) { return 0; + } for(auto &type : getElementTypes()) { - if(boost::equals(element.getType(), type)) + if(boost::equals(element.getType(), type)) { return type[0]; + } } return 0; } +// The elementType function can take a char *, in which case it tries a sequence of checks to +// see what it got. +// 1) Check to see if it is an indexedName, and if so directly look up the type +// 2) If not: +// a) Remove any element map prefix that is present +// b) See if the name contains a dot: +// i) If yes, create a MappedName from the part before the dot, and set the type to the +// part after the dot +// ii) If no, create a MappedName from the whole name +// c) Try to get the elementType based on the MappedName. Return it if found +// 3) Check to make sure the discovered type is in the list of types, and return its first +// character if so. char ComplexGeoData::elementType(const char *name) const { - if(!name) + if(!name) { return 0; + } const char *type = nullptr; IndexedName element(name, getElementTypes()); - if (element) + if (element) { type = element.getType(); + } else { const char * mapped = isMappedElement(name); - if (mapped) + if (mapped) { name = mapped; + } - MappedName n; + MappedName mappedName; const char *dot = strchr(name,'.'); if(dot) { - n = MappedName(name, dot-name); + mappedName = MappedName(name, dot-name); type = dot+1; } - else - n = MappedName::fromRawData(name); - char res = elementType(n); - if (res) + else { + mappedName = MappedName::fromRawData(name); + } + char res = elementType(mappedName); + if (res != 0) { return res; + } } if(type && type[0]) { - for(auto &t : getElementTypes()) { - if(boost::starts_with(type, t)) + for(auto &elementTypes : getElementTypes()) { + if(boost::starts_with(type, elementTypes)) { return type[0]; + } } } return 0; } -MappedName ComplexGeoData::renameDuplicateElement(int index, - const IndexedName & element, - const IndexedName & element2, - const MappedName & name, - ElementIDRefs &sids) -{ - std::ostringstream ss; - ss << ELEMENT_MAP_PREFIX << 'D' << std::hex << index; - MappedName renamed(name); - this->elementMap()->encodeElementName(element.getType()[0],renamed,ss,&sids,Tag); - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) - FC_WARN("duplicate element mapping '" << name << " -> " << renamed << ' ' - << element << '/' << element2); - return renamed; -} +// NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) diff --git a/src/App/ComplexGeoData.h b/src/App/ComplexGeoData.h index e7aa240e94..dd1b81ff62 100644 --- a/src/App/ComplexGeoData.h +++ b/src/App/ComplexGeoData.h @@ -24,8 +24,8 @@ ***************************************************************************/ -#ifndef _AppComplexGeoData_h_ -#define _AppComplexGeoData_h_ +#ifndef APP_COMPLEX_GEO_DATA_H +#define APP_COMPLEX_GEO_DATA_H #include #include @@ -45,7 +45,7 @@ namespace Base { class Placement; class Rotation; -template class BoundBox3; +template class BoundBox3;// NOLINT using BoundBox3d = BoundBox3; } @@ -55,12 +55,12 @@ namespace Data struct MappedChildElements; /** Segments - * Subelement type of the ComplexGeoData type + * Sub-element type of the ComplexGeoData type * It is used to split an object in further sub-parts. */ class AppExport Segment: public Base::BaseClass { - TYPESYSTEM_HEADER_WITH_OVERRIDE(); + TYPESYSTEM_HEADER_WITH_OVERRIDE();// NOLINT public: ~Segment() override = default; @@ -72,7 +72,7 @@ public: */ class AppExport ComplexGeoData: public Base::Persistence, public Base::Handled { - TYPESYSTEM_HEADER_WITH_OVERRIDE(); + TYPESYSTEM_HEADER_WITH_OVERRIDE();// NOLINT public: struct Line {uint32_t I1; uint32_t I2;}; @@ -87,17 +87,17 @@ public: /// Destructor ~ComplexGeoData() override; - /** @name Subelement management */ + /** @name Sub-element management */ //@{ /** Sub type list - * List of different subelement types - * its NOT a list of the subelements itself + * List of different sub-element types + * its NOT a list of the sub-elements itself */ virtual std::vector getElementTypes() const=0; virtual unsigned long countSubElements(const char* Type) const=0; - /// get the subelement by type and number + /// get the sub-element by type and number virtual Segment* getSubElement(const char* Type, unsigned long) const=0; - /// get subelement by combined name + /// get sub-element by combined name virtual Segment* getSubElementByName(const char* Name) const; /** Get lines from segment */ virtual void getLinesFromSubElement( @@ -203,8 +203,8 @@ public: /** Return a pair of indexed name and mapped name * * @param name: the input name. - * @param sid: optional output of and App::StringID involved forming this - * mapped name + * @param sid: optional output of any App::StringID involved in forming + * this mapped name * @param copy: if true, copy the name string, or else use it as constant * string, and caller must make sure the memory is not freed. * @@ -221,9 +221,6 @@ public: ElementIDRefs *sid = nullptr, bool copy = false) const; - /** Get mapped element with a given prefix */ - std::vector getElementNamesWithPrefix(const char *prefix) const; - /** Get mapped element names * * @param element: original element name with \c Type + \c Index @@ -237,7 +234,9 @@ public: getElementMappedNames(const IndexedName & element, bool needUnmapped=false) const; /// Append the Tag (if and only if it is non zero) into the element map - virtual void reTagElementMap(long tag, App::StringHasherRef hasher, const char *postfix=0) { + virtual void reTagElementMap(long tag, + App::StringHasherRef hasher, + const char *postfix=nullptr) { (void)tag; (void)hasher; (void)postfix; @@ -269,37 +268,13 @@ public: /// Get the current element map size size_t getElementMapSize(bool flush=true) const; - /// Check if the given subname only contains an element name - static bool isElementName(const char *subname) { - return subname && *subname && findElementName(subname)==subname; + /// Check if the given sub-name only contains an element name + static bool isElementName(const char *subName) { + return (subName != nullptr) && (*subName != 0) && findElementName(subName)==subName; } - /** Element trace callback - * - * The callback has the following call signature - * (const std::string &name, size_t offset, long encodedTag, long tag) -> bool - * - * @param name: the current element name. - * @param offset: the offset skipping the encoded element name for the next iteration. - * @param encodedTag: the tag encoded inside the current element, which is usually the tag - * of the previous step in the shape history. - * @param tag: the tag of the current shape element. - * - * @sa traceElement() - */ - using TraceCallback = std::function; - - /** Iterate through the history of the give element name with a given callback - * - * @param name: the input element name - * @param cb: trace callback with call signature. - * @sa TraceCallback - */ - void traceElement(const MappedName &name, TraceCallback cb) const; - /** Flush an internal buffering for element mapping */ virtual void flushElementMap() const; - virtual unsigned long getElementMapReserve() const { return 0; } //@} protected: @@ -368,11 +343,6 @@ public: mutable App::StringHasherRef Hasher; protected: - virtual MappedName renameDuplicateElement(int index, - const IndexedName & element, - const IndexedName & element2, - const MappedName & name, - ElementIDRefs &sids); /// from local to outside inline Base::Vector3d transformToOutside(const Base::Vector3f& vec) const diff --git a/src/App/ElementMap.cpp b/src/App/ElementMap.cpp index cc22f6b569..3be76b9973 100644 --- a/src/App/ElementMap.cpp +++ b/src/App/ElementMap.cpp @@ -1,7 +1,9 @@ #include "PreCompiled.h" #ifndef _PreComp_ -# include -# include +#include +#ifndef FC_DEBUG +#include +#endif #endif #include "ElementMap.h" @@ -10,11 +12,11 @@ #include "App/Application.h" #include "Base/Console.h" -# include -# include +#include +#include -FC_LOG_LEVEL_INIT("ElementMap", true, 2); +FC_LOG_LEVEL_INIT("ElementMap", true, 2);// NOLINT namespace Data { @@ -37,27 +39,28 @@ namespace Data // any shape sharing is lost. But again, we do want to keep separate shape files // because of partial loading. The same technique used here can be applied to // restore shape sharing. -static std::unordered_map _ElementMapToId; -static std::unordered_map _IdToElementMap; +static std::unordered_map _elementMapToId; +static std::unordered_map _idToElementMap; -void ElementMap::init() { +void ElementMap::init() +{ static bool inited; if (!inited) { inited = true; ::App::GetApplication().signalStartSaveDocument.connect( [](const ::App::Document&, const std::string&) { - _ElementMapToId.clear(); + _elementMapToId.clear(); }); ::App::GetApplication().signalFinishSaveDocument.connect( [](const ::App::Document&, const std::string&) { - _ElementMapToId.clear(); + _elementMapToId.clear(); }); ::App::GetApplication().signalStartRestoreDocument.connect([](const ::App::Document&) { - _IdToElementMap.clear(); + _idToElementMap.clear(); }); ::App::GetApplication().signalFinishRestoreDocument.connect([](const ::App::Document&) { - _IdToElementMap.clear(); + _idToElementMap.clear(); }); } } @@ -68,131 +71,145 @@ ElementMap::ElementMap() } -void ElementMap::beforeSave(const ::App::StringHasherRef& hasher) const +void ElementMap::beforeSave(const ::App::StringHasherRef& hasherRef) const { - unsigned& id = _ElementMapToId[this]; - if (!id) - id = _ElementMapToId.size(); + unsigned& id = _elementMapToId[this]; + if (id == 0U) { + id = _elementMapToId.size(); + } this->_id = id; - for (auto& v : this->indexedNames) { - for (const MappedNameRef& ref : v.second.names) { - for (const MappedNameRef* r = &ref; r; r = r->next.get()) { - for (const ::App::StringIDRef& sid : r->sids) { - if (sid.isFromSameHasher(hasher)) + for (auto& indexedName : this->indexedNames) { + for (const MappedNameRef& mappedName : indexedName.second.names) { + for (const MappedNameRef* ref = &mappedName; ref; ref = ref->next.get()) { + for (const ::App::StringIDRef& sid : ref->sids) { + if (sid.isFromSameHasher(hasherRef)) { sid.mark(); + } } } } - for (auto& vv : v.second.children) { - if (vv.second.elementMap) - vv.second.elementMap->beforeSave(hasher); - for (auto& sid : vv.second.sids) { - if (sid.isFromSameHasher(hasher)) + for (auto& childPair : indexedName.second.children) { + if (childPair.second.elementMap) { + childPair.second.elementMap->beforeSave(hasherRef); + } + for (auto& sid : childPair.second.sids) { + if (sid.isFromSameHasher(hasherRef)) { sid.mark(); + } } } } } -void ElementMap::save(std::ostream& s, int index, +void ElementMap::save(std::ostream& stream, int index, const std::map& childMapSet, const std::map& postfixMap) const { - s << "\nElementMap " << index << ' ' << this->_id << ' ' << this->indexedNames.size() << '\n'; + stream << "\nElementMap " << index << ' ' << this->_id << ' ' << this->indexedNames.size() + << '\n'; - for (auto& v : this->indexedNames) { - s << '\n' << v.first << '\n'; + for (auto& indexedName : this->indexedNames) { + stream << '\n' << indexedName.first << '\n'; - s << "\nChildCount " << v.second.children.size() << '\n'; - for (auto& vv : v.second.children) { + stream << "\nChildCount " << indexedName.second.children.size() << '\n'; + for (auto& vv : indexedName.second.children) { auto& child = vv.second; int mapIndex = 0; if (child.elementMap) { auto it = childMapSet.find(child.elementMap.get()); - if (it == childMapSet.end() || it->second == 0) - FC_ERR("Invalid child element map"); - else + if (it == childMapSet.end() || it->second == 0) { + FC_ERR("Invalid child element map");// NOLINT + } + else { mapIndex = it->second; + } } - s << child.indexedName.getIndex() << ' ' << child.offset << ' ' << child.count << ' ' - << child.tag << ' ' << mapIndex << ' '; - s.write(child.postfix.constData(), child.postfix.size()); - s << ' ' << '0'; + stream << child.indexedName.getIndex() << ' ' << child.offset << ' ' << child.count + << ' ' << child.tag << ' ' << mapIndex << ' '; + stream.write(child.postfix.constData(), child.postfix.size()); + stream << ' ' << '0'; for (auto& sid : child.sids) { - if (sid.isMarked()) - s << '.' << sid.value(); + if (sid.isMarked()) { + stream << '.' << sid.value(); + } } - s << '\n'; + stream << '\n'; } - s << "\nNameCount " << v.second.names.size() << '\n'; - if (v.second.names.empty()) + stream << "\nNameCount " << indexedName.second.names.size() << '\n'; + if (indexedName.second.names.empty()) { continue; + } - boost::io::ios_flags_saver ifs(s); - s << std::hex; + boost::io::ios_flags_saver ifs(stream); + stream << std::hex; - for (auto& ref : v.second.names) { - for (auto r = &ref; r; r = r->next.get()) { - if (!r->name) + for (auto& dequeueOfMappedNameRef : indexedName.second.names) { + for (auto ref = &dequeueOfMappedNameRef; ref; ref = ref->next.get()) { + if (!ref->name) { break; + } - ::App::StringID::IndexID prefixid; - prefixid.id = 0; - IndexedName idx(r->name.dataBytes()); + ::App::StringID::IndexID prefixID {}; + prefixID.id = 0; + IndexedName idx(ref->name.dataBytes()); bool printName = true; if (idx) { - auto key = QByteArray::fromRawData(idx.getType(), qstrlen(idx.getType())); + auto key = QByteArray::fromRawData(idx.getType(), + static_cast(qstrlen(idx.getType()))); auto it = postfixMap.find(key); if (it != postfixMap.end()) { - s << ':' << it->second << '.' << idx.getIndex(); + stream << ':' << it->second << '.' << idx.getIndex(); printName = false; } } else { - prefixid = ::App::StringID::fromString(r->name.dataBytes()); - if (prefixid.id) { - for (auto& sid : r->sids) { - if (sid.isMarked() && sid.value() == prefixid.id) { - s << '$'; - s.write(r->name.dataBytes().constData(), - r->name.dataBytes().size()); + prefixID = ::App::StringID::fromString(ref->name.dataBytes()); + if (prefixID.id != 0) { + for (auto& sid : ref->sids) { + if (sid.isMarked() && sid.value() == prefixID.id) { + stream << '$'; + stream.write(ref->name.dataBytes().constData(), + ref->name.dataBytes().size()); printName = false; break; } } - if (printName) - prefixid.id = 0; + if (printName) { + prefixID.id = 0; + } } } if (printName) { - s << ';'; - s.write(r->name.dataBytes().constData(), r->name.dataBytes().size()); + stream << ';'; + stream.write(ref->name.dataBytes().constData(), ref->name.dataBytes().size()); } - const QByteArray& postfix = r->name.postfixBytes(); - if (postfix.isEmpty()) - s << ".0"; + const QByteArray& postfix = ref->name.postfixBytes(); + if (postfix.isEmpty()) { + stream << ".0"; + } else { auto it = postfixMap.find(postfix); assert(it != postfixMap.end()); - s << '.' << it->second; + stream << '.' << it->second; } - for (auto& sid : r->sids) { - if (sid.isMarked() && sid.value() != prefixid.id) - s << '.' << sid.value(); + for (auto& sid : ref->sids) { + if (sid.isMarked() && sid.value() != prefixID.id) { + stream << '.' << sid.value(); + } } - s << ' '; + stream << ' '; } - s << "0\n"; + stream << "0\n"; } } - s << "\nEndMap\n"; + stream << "\nEndMap\n"; } -void ElementMap::save(std::ostream& s) const +void ElementMap::save(std::ostream& stream) const { std::map childMapSet; std::vector childMaps; @@ -201,72 +218,79 @@ void ElementMap::save(std::ostream& s) const collectChildMaps(childMapSet, childMaps, postfixMap, postfixes); - s << this->_id << " PostfixCount " << postfixes.size() << '\n'; - for (auto& p : postfixes) { - s.write(p.constData(), p.size()); - s << '\n'; + stream << this->_id << " PostfixCount " << postfixes.size() << '\n'; + for (auto& postfix : postfixes) { + stream.write(postfix.constData(), postfix.size()); + stream << '\n'; } int index = 0; - s << "\nMapCount " << childMaps.size() << '\n'; - for (auto& elementMap : childMaps) - elementMap->save(s, ++index, childMapSet, postfixMap); + stream << "\nMapCount " << childMaps.size() << '\n'; + for (auto& elementMap : childMaps) { + elementMap->save(stream, ++index, childMapSet, postfixMap); + } } -ElementMapPtr ElementMap::restore(::App::StringHasherRef hasher, std::istream& s) +ElementMapPtr ElementMap::restore(::App::StringHasherRef hasherRef, std::istream& stream) { const char* msg = "Invalid element map"; - unsigned id; + unsigned id = 0; int count = 0; std::string tmp; - if (!(s >> id >> tmp >> count) || tmp != "PostfixCount") - FC_THROWM(Base::RuntimeError, msg); + if (!(stream >> id >> tmp >> count) || tmp != "PostfixCount") { + FC_THROWM(Base::RuntimeError, msg);// NOLINT + } - auto& map = _IdToElementMap[id]; - if (map) + auto& map = _idToElementMap[id]; + if (map) { return map; + } std::vector postfixes; postfixes.reserve(count); for (int i = 0; i < count; ++i) { postfixes.emplace_back(); - s >> postfixes.back(); + stream >> postfixes.back(); } std::vector childMaps; count = 0; - if (!(s >> tmp >> count) || tmp != "MapCount" || count == 0) - FC_THROWM(Base::RuntimeError, msg); + if (!(stream >> tmp >> count) || tmp != "MapCount" || count == 0) { + FC_THROWM(Base::RuntimeError, msg);// NOLINT + } childMaps.reserve(count - 1); for (int i = 0; i < count - 1; ++i) { childMaps.push_back( - std::make_shared()->restore(hasher, s, childMaps, postfixes)); + std::make_shared()->restore(hasherRef, stream, childMaps, postfixes)); } - return restore(hasher, s, childMaps, postfixes); + return restore(hasherRef, stream, childMaps, postfixes); } -ElementMapPtr ElementMap::restore(::App::StringHasherRef hasher, std::istream& s, +ElementMapPtr ElementMap::restore(::App::StringHasherRef hasherRef, std::istream& stream, std::vector& childMaps, const std::vector& postfixes) { const char* msg = "Invalid element map"; + const int hexBase {16}; + const int decBase {10}; std::string tmp; int index = 0; int typeCount = 0; unsigned id = 0; - if (!(s >> tmp >> index >> id >> typeCount) || tmp != "ElementMap") - FC_THROWM(Base::RuntimeError, msg); + if (!(stream >> tmp >> index >> id >> typeCount) || tmp != "ElementMap") { + FC_THROWM(Base::RuntimeError, msg);// NOLINT + } - auto& map = _IdToElementMap[id]; + auto& map = _idToElementMap[id]; if (map) { - do { - if (!std::getline(s, tmp)) - FC_THROWM(Base::RuntimeError, "unexpected end of child element map"); - } while (tmp != "EndMap"); + while (tmp != "EndMap") { + if (!std::getline(stream, tmp)) { + FC_THROWM(Base::RuntimeError, "unexpected end of child element map");// NOLINT + } + } return map; } - map = shared_from_this(); //FIXME does nothing? const char* hasherWarn = nullptr; const char* hasherIDWarn = nullptr; @@ -275,164 +299,193 @@ ElementMapPtr ElementMap::restore(::App::StringHasherRef hasher, std::istream& s std::vector tokens; for (int i = 0; i < typeCount; ++i) { - int count; - if (!(s >> tmp)) - FC_THROWM(Base::RuntimeError, "missing element type"); + int outerCount = 0; + if (!(stream >> tmp)) { + FC_THROWM(Base::RuntimeError, "missing element type");// NOLINT + } IndexedName idx(tmp.c_str(), 1); - if (!(s >> tmp >> count) || tmp != "ChildCount") - FC_THROWM(Base::RuntimeError, "missing element child count"); + if (!(stream >> tmp >> outerCount) || tmp != "ChildCount") { + FC_THROWM(Base::RuntimeError, "missing element child count");// NOLINT + } auto& indices = this->indexedNames[idx.getType()]; - for (int j = 0; j < count; ++j) { - int cindex; - int offset; - int count; - long tag; - int mapIndex; - if (!(s >> cindex >> offset >> count >> tag >> mapIndex >> tmp)) - FC_THROWM(Base::RuntimeError, "Invalid element child"); - if (cindex < 0) - FC_THROWM(Base::RuntimeError, "Invalid element child index"); - if (offset < 0) - FC_THROWM(Base::RuntimeError, "Invalid element child offset"); - if (mapIndex >= index || mapIndex < 0 || mapIndex > (int)childMaps.size()) - FC_THROWM(Base::RuntimeError, "Invalid element child map index"); - auto& child = indices.children[cindex + offset + count]; - child.indexedName = IndexedName::fromConst(idx.getType(), cindex); + for (int j = 0; j < outerCount; ++j) { + int cIndex = 0; + int offset = 0; + int count = 0; + long tag = 0; + int mapIndex = 0; + if (!(stream >> cIndex >> offset >> count >> tag >> mapIndex >> tmp)) { + FC_THROWM(Base::RuntimeError, "Invalid element child");// NOLINT + } + if (cIndex < 0) { + FC_THROWM(Base::RuntimeError, "Invalid element child index");// NOLINT + } + if (offset < 0) { + FC_THROWM(Base::RuntimeError, "Invalid element child offset");// NOLINT + } + if (mapIndex >= index || mapIndex < 0 || mapIndex > (int)childMaps.size()) { + FC_THROWM(Base::RuntimeError, "Invalid element child map index");// NOLINT + } + auto& child = indices.children[cIndex + offset + count]; + child.indexedName = IndexedName::fromConst(idx.getType(), cIndex); child.offset = offset; child.count = count; child.tag = tag; - if (mapIndex > 0) + if (mapIndex > 0) { child.elementMap = childMaps[mapIndex - 1]; - else + } + else { child.elementMap = nullptr; + } child.postfix = tmp.c_str(); this->childElements[child.postfix].childMap = &child; this->childElementSize += child.count; - if (!(s >> tmp)) - FC_THROWM(Base::RuntimeError, "Invalid element child string id"); + if (!(stream >> tmp)) { + FC_THROWM(Base::RuntimeError, "Invalid element child string id");// NOLINT + } tokens.clear(); boost::split(tokens, tmp, boost::is_any_of(".")); if (tokens.size() > 1) { - child.sids.reserve(tokens.size() - 1); + child.sids.reserve(static_cast(tokens.size()) - 1); for (unsigned k = 1; k < tokens.size(); ++k) { // The element child string ID is saved as decimal // instead of hex by accident. To simplify maintenance // of backward compatibility, it is not corrected, and // just restored as decimal here. - // - // long n = strtol(tokens[k].c_str(), nullptr, 16); - long n = strtol(tokens[k].c_str(), nullptr, 10); - auto sid = hasher->getID(n); - if (!sid) + long childID = strtol(tokens[k].c_str(), nullptr, decBase); + auto sid = hasherRef->getID(childID); + if (!sid) { childSIDWarn = "Missing element child string id"; - else + } + else { child.sids.push_back(sid); + } } } } - if (!(s >> tmp >> count) || tmp != "NameCount") - FC_THROWM(Base::RuntimeError, "missing element name count"); + if (!(stream >> tmp >> outerCount) || tmp != "NameCount") { + FC_THROWM(Base::RuntimeError, "missing element name outerCount");// NOLINT + } - boost::io::ios_flags_saver ifs(s); - s >> std::hex; + boost::io::ios_flags_saver ifs(stream); + stream >> std::hex; - indices.names.resize(count); - for (int j = 0; j < count; ++j) { + indices.names.resize(outerCount); + for (int j = 0; j < outerCount; ++j) { idx.setIndex(j); auto* ref = &indices.names[j]; - int k = 0; - while (1) { - if (!(s >> tmp)) - FC_THROWM(Base::RuntimeError, "Failed to read element name"); - if (tmp == "0") + int innerCount = 0; + while (true) { + if (!(stream >> tmp)) { + FC_THROWM(Base::RuntimeError, "Failed to read element name");// NOLINT + } + if (tmp == "0") { break; - if (k++ != 0) { - ref->next.reset(new MappedNameRef); + } + if (innerCount++ != 0) { + ref->next = std::make_unique(); ref = ref->next.get(); } tokens.clear(); boost::split(tokens, tmp, boost::is_any_of(".")); - if (tokens.size() < 2) - FC_THROWM(Base::RuntimeError, "Invalid element entry"); + if (tokens.size() < 2) { + FC_THROWM(Base::RuntimeError, "Invalid element entry");// NOLINT + } int offset = 1; - ::App::StringID::IndexID prefixid; - prefixid.id = 0; + ::App::StringID::IndexID prefixID {}; + prefixID.id = 0; switch (tokens[0][0]) { case ':': { - if (tokens.size() < 3) - FC_THROWM(Base::RuntimeError, "Invalid element entry"); + if (tokens.size() < 3) { + FC_THROWM(Base::RuntimeError, "Invalid element entry");// NOLINT + } ++offset; - long n = strtol(tokens[0].c_str() + 1, nullptr, 16); - if (n <= 0 || n > (int)postfixes.size()) - FC_THROWM(Base::RuntimeError, "Invalid element name index"); - long m = strtol(tokens[1].c_str(), nullptr, 16); - ref->name = MappedName(IndexedName::fromConst(postfixes[n - 1].c_str(), m)); + long elementNameIndex = strtol(tokens[0].c_str() + 1, nullptr, hexBase); + if (elementNameIndex <= 0 || elementNameIndex > (int)postfixes.size()) { + FC_THROWM(Base::RuntimeError, "Invalid element name index");// NOLINT + } + long elementIndex = strtol(tokens[1].c_str(), nullptr, hexBase); + ref->name = MappedName( + IndexedName::fromConst(postfixes[elementNameIndex - 1].c_str(), + static_cast(elementIndex))); break; } case '$': ref->name = MappedName(tokens[0].c_str() + 1); - prefixid = ::App::StringID::fromString(ref->name.dataBytes()); + prefixID = ::App::StringID::fromString(ref->name.dataBytes()); break; case ';': ref->name = MappedName(tokens[0].c_str() + 1); break; default: - FC_THROWM(Base::RuntimeError, "Invalid element name marker"); + FC_THROWM(Base::RuntimeError, "Invalid element name marker");// NOLINT } if (tokens[offset] != "0") { - long n = strtol(tokens[offset].c_str(), nullptr, 16); - if (n <= 0 || n > (int)postfixes.size()) + long postfixIndex = strtol(tokens[offset].c_str(), nullptr, hexBase); + if (postfixIndex <= 0 || postfixIndex > (int)postfixes.size()) { postfixWarn = "Invalid element postfix index"; - else - ref->name += postfixes[n - 1]; + } + else { + ref->name += postfixes[postfixIndex - 1]; + } } this->mappedNames.emplace(ref->name, idx); - if (!hasher) { - if (offset + 1 < (int)tokens.size()) - hasherWarn = "No hasher"; + if (!hasherRef) { + if (offset + 1 < (int)tokens.size()) { + hasherWarn = "No hasherRef"; + } continue; } - ref->sids.reserve(tokens.size() - offset - 1 + prefixid.id ? 1 : 0); - if (prefixid.id) { - auto sid = hasher->getID(prefixid.id); - if (!sid) + ref->sids.reserve((tokens.size() - offset - 1 + prefixID.id) != 0U ? 1 : 0); + if (prefixID.id != 0) { + auto sid = hasherRef->getID(prefixID.id); + if (!sid) { hasherIDWarn = "Missing element name prefix id"; - else + } + else { ref->sids.push_back(sid); + } } for (int l = offset + 1; l < (int)tokens.size(); ++l) { - long id = strtol(tokens[l].c_str(), nullptr, 16); - auto sid = hasher->getID(id); - if (!sid) + long readID = strtol(tokens[l].c_str(), nullptr, hexBase); + auto sid = hasherRef->getID(readID); + if (!sid) { hasherIDWarn = "Invalid element name string id"; - else + } + else { ref->sids.push_back(sid); + } } } } } - if (hasherWarn) - FC_WARN(hasherWarn); - if (hasherIDWarn) - FC_WARN(hasherIDWarn); - if (postfixWarn) - FC_WARN(postfixWarn); - if (childSIDWarn) - FC_WARN(childSIDWarn); + if (hasherWarn) { + FC_WARN(hasherWarn);// NOLINT + } + if (hasherIDWarn) { + FC_WARN(hasherIDWarn);// NOLINT + } + if (postfixWarn) { + FC_WARN(postfixWarn);// NOLINT + } + if (childSIDWarn) { + FC_WARN(childSIDWarn);// NOLINT + } - if (!(s >> tmp) || tmp != "EndMap") - FC_THROWM(Base::RuntimeError, "unexpected end of child element map"); + if (!(stream >> tmp) || tmp != "EndMap") { + FC_THROWM(Base::RuntimeError, "unexpected end of child element map");// NOLINT + } return shared_from_this(); } @@ -442,38 +495,41 @@ MappedName ElementMap::addName(MappedName& name, const IndexedName& idx, const E { if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { if (name.find("#") >= 0 && name.findTagInElementName() < 0) { - FC_ERR("missing tag postfix " << name); + FC_ERR("missing tag postfix " << name);// NOLINT } } - do { - if (overwrite) + while (true) { + if (overwrite) { erase(idx); + } auto ret = mappedNames.insert(std::make_pair(name, idx)); if (ret.second) { // element just inserted did not exist yet in the map ret.first->first.compact();// FIXME see MappedName.cpp mappedRef(idx).append(ret.first->first, sids); - FC_TRACE(idx << " -> " << name); + FC_TRACE(idx << " -> " << name);// NOLINT return ret.first->first; } if (ret.first->second == idx) { - FC_TRACE("duplicate " << idx << " -> " << name); + FC_TRACE("duplicate " << idx << " -> " << name);// NOLINT return ret.first->first; } if (!overwrite) { - if (existing) + if (existing) { *existing = ret.first->second; - return MappedName(); + } + return {}; } erase(ret.first->first); - } while (true); + }; } void ElementMap::addPostfix(const QByteArray& postfix, std::map& postfixMap, std::vector& postfixes) { - if (postfix.isEmpty()) + if (postfix.isEmpty()) { return; + } auto res = postfixMap.insert(std::make_pair(postfix, 0)); if (res.second) { postfixes.push_back(postfix); @@ -481,51 +537,57 @@ void ElementMap::addPostfix(const QByteArray& postfix, std::map } } -MappedName ElementMap::setElementName(const IndexedName& element, - const MappedName& name, - long masterTag, - const ElementIDRefs* sid, - bool overwrite) +MappedName ElementMap::setElementName(const IndexedName& element, const MappedName& name, + long masterTag, const ElementIDRefs* sid, bool overwrite) { - if (!element) + if (!element) { throw Base::ValueError("Invalid input"); + } if (!name) { erase(element); - return MappedName(); + return {}; } for (int i = 0, count = name.size(); i < count; ++i) { - char c = name[i]; - if (c == '.' || std::isspace((int)c)) - FC_THROWM(Base::RuntimeError, "Illegal character in mapped name: " << name); + char check = name[i]; + if (check == '.' || (std::isspace((int)check) != 0)) { + FC_THROWM(Base::RuntimeError, "Illegal character in mapped name: " << name);// NOLINT + } } - for (const char* s = element.getType(); *s; ++s) { - char c = *s; - if (c == '.' || std::isspace((int)c)) - FC_THROWM(Base::RuntimeError, "Illegal character in element name: " << element); + for (const char* readChar = element.getType(); *readChar != 0; ++readChar) { + char check = *readChar; + if (check == '.' || (std::isspace((int)check) != 0)) { + FC_THROWM(Base::RuntimeError,// NOLINT + "Illegal character in element name: " << element); + } } ElementIDRefs _sid; - if (!sid) + if (!sid) { sid = &_sid; + } std::ostringstream ss; - Data::MappedName n(name); + Data::MappedName mappedName(name); for (int i = 0;;) { IndexedName existing; - MappedName res = this->addName(n, element, *sid, overwrite, &existing); - if (res) + MappedName res = this->addName(mappedName, element, *sid, overwrite, &existing); + if (res) { return res; - if (++i == 100) { - FC_ERR("unresolved duplicate element mapping '" << name << ' ' << element << '/' - << existing); + } + const int maxAttempts {100}; + if (++i == maxAttempts) { + FC_ERR("unresolved duplicate element mapping '"// NOLINT + << name << ' ' << element << '/' << existing); return name; } - if (sid != &_sid) + if (sid != &_sid) { _sid = *sid; - n = renameDuplicateElement(i, element, existing, name, _sid, masterTag); - if (!n) + } + mappedName = renameDuplicateElement(i, element, existing, name, _sid, masterTag); + if (!mappedName) { return name; + } sid = &_sid; } } @@ -535,60 +597,68 @@ void ElementMap::encodeElementName(char element_type, MappedName& name, std::ost ElementIDRefs* sids, long masterTag, const char* postfix, long tag, bool forceTag) const { - if (postfix && postfix[0]) { - if (!boost::starts_with(postfix, ELEMENT_MAP_PREFIX)) + if (postfix && (postfix[0] != 0)) { + if (!boost::starts_with(postfix, ELEMENT_MAP_PREFIX)) { ss << ELEMENT_MAP_PREFIX; + } ss << postfix; } long inputTag = 0; - if (!forceTag && !ss.tellp()) { - if (!tag || tag == masterTag) + if (!forceTag && (ss.tellp() == 0)) { + if ((tag == 0) || tag == masterTag) { return; + } name.findTagInElementName(&inputTag, nullptr, nullptr, nullptr, true); - if (inputTag == tag) + if (inputTag == tag) { return; + } } - else if (!tag || (!forceTag && tag == masterTag)) { + else if ((tag == 0) || (!forceTag && tag == masterTag)) { int pos = name.findTagInElementName(&inputTag, nullptr, nullptr, nullptr, true); - if (inputTag) { + if (inputTag != 0) { tag = inputTag; // About to encode the same tag used last time. This usually means - // the owner object is doing multi step modeling. Let's not - // recursively encode the same tag too many time. It will be a + // the owner object is doing multistep modeling. Let's not + // recursively encode the same tag too many times. It will be a // waste of memory, because the intermediate shapes has no // corresponding objects, so no real value for history tracing. // // On the other hand, we still need to distinguish the original name // from the input object from the element name of the intermediate // shapes. So we limit ourselves to encode only one extra level - // using the same tag. In order to do that, we need to dehash the + // using the same tag. In order to do that, we need to de-hash the // previous level name, and check for its tag. - Data::MappedName n(name, 0, pos); - Data::MappedName prev = dehashElementName(n); + Data::MappedName mappedName(name, 0, pos); + Data::MappedName prev = dehashElementName(mappedName); long prevTag = 0; prev.findTagInElementName(&prevTag, nullptr, nullptr, nullptr, true); - if (prevTag == inputTag || prevTag == -inputTag) - name = n; + if (prevTag == inputTag || prevTag == -inputTag) { + name = mappedName; + } } } if (sids && this->hasher) { name = hashElementName(name, *sids); - if (!forceTag && !tag && ss.tellp()) + if (!forceTag && (tag == 0) && (ss.tellp() != 0)) { forceTag = true; + } } - if (forceTag || tag) { + if (forceTag || (tag != 0)) { assert(element_type); - int pos = ss.tellp(); + auto pos = ss.tellp(); boost::io::ios_flags_saver ifs(ss); ss << POSTFIX_TAG << std::hex; - if (tag < 0) + if (tag < 0) { ss << '-' << -tag; - else if (tag) + } + else if (tag != 0) { ss << tag; + } assert(pos >= 0); - if (pos != 0) + if (pos != 0) { ss << ':' << pos; + } ss << ',' << element_type; } name += ss.str(); @@ -596,10 +666,12 @@ void ElementMap::encodeElementName(char element_type, MappedName& name, std::ost MappedName ElementMap::hashElementName(const MappedName& name, ElementIDRefs& sids) const { - if (!this->hasher || !name) + if (!this->hasher || !name) { return name; - if (name.find(ELEMENT_MAP_PREFIX) < 0) + } + if (name.find(ELEMENT_MAP_PREFIX) < 0) { return name; + } App::StringIDRef sid = this->hasher->getID(name, sids); const auto& related = sid.relatedIDs(); if (related == sids) { @@ -609,9 +681,10 @@ MappedName ElementMap::hashElementName(const MappedName& name, ElementIDRefs& si else { ElementIDRefs tmp; tmp.push_back(sid); - for (auto& s : sids) { - if (related.indexOf(s) < 0) - tmp.push_back(s); + for (auto& checkSID : sids) { + if (related.indexOf(checkSID) < 0) { + tmp.push_back(checkSID); + } } sids = tmp; } @@ -620,42 +693,47 @@ MappedName ElementMap::hashElementName(const MappedName& name, ElementIDRefs& si MappedName ElementMap::dehashElementName(const MappedName& name) const { - if (name.empty()) + if (name.empty()) { return name; - if (!this->hasher) + } + if (!this->hasher) { return name; + } auto id = App::StringID::fromString(name.toRawBytes()); - if (!id) + if (!id) { return name; + } auto sid = this->hasher->getID(id); if (!sid) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_TRACE)) - FC_WARN("failed to find hash id " << id); - else - FC_LOG("failed to find hash id " << id); + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_TRACE)) { + FC_WARN("failed to find hash id " << id);// NOLINT + } + else { + FC_LOG("failed to find hash id " << id);// NOLINT + } return name; } if (sid.isHashed()) { - FC_LOG("cannot dehash id " << id); + FC_LOG("cannot de-hash id " << id);// NOLINT return name; } MappedName ret( sid.toString());// FIXME .toString() was missing in original function. is this correct? - FC_TRACE("dehash " << name << " -> " << ret); + FC_TRACE("de-hash " << name << " -> " << ret);// NOLINT return ret; } MappedName ElementMap::renameDuplicateElement(int index, const IndexedName& element, const IndexedName& element2, const MappedName& name, - ElementIDRefs& sids, long masterTag) + ElementIDRefs& sids, long masterTag) const { - int idx; + int idx {0}; #ifdef FC_DEBUG idx = index; #else static std::random_device _RD; static std::mt19937 _RGEN(_RD()); - static std::uniform_int_distribution<> _RDIST(1,10000); + static std::uniform_int_distribution<> _RDIST(1, 10000); (void)index; idx = _RDIST(_RGEN); #endif @@ -663,38 +741,42 @@ MappedName ElementMap::renameDuplicateElement(int index, const IndexedName& elem ss << ELEMENT_MAP_PREFIX << 'D' << std::hex << idx; MappedName renamed(name); encodeElementName(element.getType()[0], renamed, ss, &sids, masterTag); - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) - FC_WARN("duplicate element mapping '" << name << " -> " << renamed << ' ' << element << '/' - << element2); + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { + FC_WARN("duplicate element mapping '"// NOLINT + << name << " -> " << renamed << ' ' << element << '/' << element2); + } return renamed; } void ElementMap::erase(const MappedName& name) { auto it = this->mappedNames.find(name); - if (it == this->mappedNames.end()) + if (it == this->mappedNames.end()) { return; + } MappedNameRef* ref = findMappedRef(it->second); - if (!ref) + if (!ref) { return; + } ref->erase(name); this->mappedNames.erase(it); - return; } void ElementMap::erase(const IndexedName& idx) { auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return; + } auto& indices = iter->second; - if (idx.getIndex() >= (int)indices.names.size()) + if (idx.getIndex() >= (int)indices.names.size()) { return; + } auto& ref = indices.names[idx.getIndex()]; - for (auto* r = &ref; r; r = r->next.get()) - this->mappedNames.erase(r->name); + for (auto* nameRef = &ref; nameRef; nameRef = nameRef->next.get()) { + this->mappedNames.erase(nameRef->name); + } ref.clear(); - return; } unsigned long ElementMap::size() const @@ -709,27 +791,32 @@ bool ElementMap::empty() const IndexedName ElementMap::find(const MappedName& name, ElementIDRefs* sids) const { - auto it = mappedNames.find(name); - if (it == mappedNames.end()) { - if (childElements.isEmpty()) + auto nameIter = mappedNames.find(name); + if (nameIter == mappedNames.end()) { + if (childElements.isEmpty()) { return IndexedName(); + } int len = 0; - if (name.findTagInElementName(nullptr, &len, nullptr, nullptr, false, false) < 0) + if (name.findTagInElementName(nullptr, &len, nullptr, nullptr, false, false) < 0) { return IndexedName(); + } QByteArray key = name.toRawBytes(len); auto it = this->childElements.find(key); - if (it == this->childElements.end()) + if (it == this->childElements.end()) { return IndexedName(); + } const auto& child = *it.value().childMap; IndexedName res; MappedName childName = MappedName::fromRawData(name, 0, len); - if (child.elementMap) + if (child.elementMap) { res = child.elementMap->find(childName, sids); - else + } + else { res = childName.toIndexedName(); + } if (res && boost::equals(res.getType(), child.indexedName.getType()) && child.indexedName.getIndex() <= res.getIndex() @@ -742,38 +829,44 @@ IndexedName ElementMap::find(const MappedName& name, ElementIDRefs* sids) const } if (sids) { - const MappedNameRef* ref = findMappedRef(it->second); + const MappedNameRef* ref = findMappedRef(nameIter->second); for (; ref; ref = ref->next.get()) { if (ref->name == name) { - if (!sids->size()) + if (sids->empty()) { *sids = ref->sids; - else + } + else { *sids += ref->sids; + } break; } } } - return it->second; + return nameIter->second; } MappedName ElementMap::find(const IndexedName& idx, ElementIDRefs* sids) const { - if (!idx) + if (!idx) { return MappedName(); + } auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return MappedName(); + } auto& indices = iter->second; if (idx.getIndex() < (int)indices.names.size()) { const MappedNameRef& ref = indices.names[idx.getIndex()]; if (ref.name) { if (sids) { - if (!sids->size()) + if (!sids->size()) { *sids = ref.sids; - else + } + else { *sids += ref.sids; + } } return ref.name; } @@ -785,41 +878,47 @@ MappedName ElementMap::find(const IndexedName& idx, ElementIDRefs* sids) const auto& child = it->second; MappedName name; IndexedName childIdx(idx.getType(), idx.getIndex() - child.offset); - if (child.elementMap) + if (child.elementMap) { name = child.elementMap->find(childIdx, sids); - else + } + else { name = MappedName(childIdx); + } if (name) { name += child.postfix; return name; } } - return MappedName(); + return {}; } std::vector> ElementMap::findAll(const IndexedName& idx) const { std::vector> res; - if (!idx) + if (!idx) { return res; + } auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return res; + } auto& indices = iter->second; if (idx.getIndex() < (int)indices.names.size()) { const MappedNameRef& ref = indices.names[idx.getIndex()]; int count = 0; - for (auto r = &ref; r; r = r->next.get()) { - if (r->name) + for (auto nameRef = &ref; nameRef; nameRef = nameRef->next.get()) { + if (nameRef->name) { ++count; + } } - if (count) { + if (count != 0) { res.reserve(count); - for (auto r = &ref; r; r = r->next.get()) { - if (r->name) - res.emplace_back(r->name, r->sids); + for (auto nameRef = &ref; nameRef; nameRef = nameRef->next.get()) { + if (nameRef->name) { + res.emplace_back(nameRef->name, nameRef->sids); + } } return res; } @@ -832,11 +931,13 @@ std::vector> ElementMap::findAll(const Inde IndexedName childIdx(idx.getType(), idx.getIndex() - child.offset); if (child.elementMap) { res = child.elementMap->findAll(childIdx); - for (auto& v : res) + for (auto& v : res) { v.first += child.postfix; + } } - else + else { res.emplace_back(MappedName(childIdx) + child.postfix, ElementIDRefs()); + } } return res; @@ -845,22 +946,26 @@ std::vector> ElementMap::findAll(const Inde const MappedNameRef* ElementMap::findMappedRef(const IndexedName& idx) const { auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return nullptr; + } auto& indices = iter->second; - if (idx.getIndex() >= (int)indices.names.size()) + if (idx.getIndex() >= (int)indices.names.size()) { return nullptr; + } return &indices.names[idx.getIndex()]; } MappedNameRef* ElementMap::findMappedRef(const IndexedName& idx) { auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return nullptr; + } auto& indices = iter->second; - if (idx.getIndex() >= (int)indices.names.size()) + if (idx.getIndex() >= (int)indices.names.size()) { return nullptr; + } return &indices.names[idx.getIndex()]; } @@ -868,8 +973,9 @@ MappedNameRef& ElementMap::mappedRef(const IndexedName& idx) { assert(idx); auto& indices = this->indexedNames[idx.getType()]; - if (idx.getIndex() >= (int)indices.names.size()) + if (idx.getIndex() >= (int)indices.names.size()) { indices.names.resize(idx.getIndex() + 1); + } return indices.names[idx.getIndex()]; } @@ -880,16 +986,18 @@ bool ElementMap::hasChildElementMap() const void ElementMap::hashChildMaps(long masterTag) { - if (childElements.empty() || !this->hasher) + if (childElements.empty() || !this->hasher) { return; + } std::ostringstream ss; for (auto& indexedNameIndexedElements : this->indexedNames) { for (auto& indexedChild : indexedNameIndexedElements.second.children) { auto& child = indexedChild.second; int len = 0; - long tag; + long tag = 0; int pos = MappedName::fromRawData(child.postfix) .findTagInElementName(&tag, &len, nullptr, nullptr, false, false); + // TODO: What is this 10? if (pos > 10) { MappedName postfix = hashElementName( MappedName::fromRawData(child.postfix.constData(), pos), child.sids); @@ -912,21 +1020,27 @@ void ElementMap::collectChildMaps(std::map& childMapSet, std::vector& postfixes) const { auto res = childMapSet.insert(std::make_pair(this, 0)); - if (!res.second) + if (!res.second) { return; + } - for (auto& v : this->indexedNames) { - addPostfix(QByteArray::fromRawData(v.first, qstrlen(v.first)), postfixMap, postfixes); + for (auto& indexedName : this->indexedNames) { + addPostfix(QByteArray::fromRawData(indexedName.first, + static_cast(qstrlen(indexedName.first))), + postfixMap, + postfixes); - for (auto& vv : v.second.children) { - auto& child = vv.second; - if (child.elementMap) + for (auto& childPair : indexedName.second.children) { + auto& child = childPair.second; + if (child.elementMap) { child.elementMap->collectChildMaps(childMapSet, childMaps, postfixMap, postfixes); + } } } - for (auto& v : this->mappedNames) - addPostfix(v.first.constPostfix(), postfixMap, postfixes); + for (auto& mappedName : this->mappedNames) { + addPostfix(mappedName.first.constPostfix(), postfixMap, postfixes); + } childMaps.push_back(this); res.first->second = (int)childMaps.size(); @@ -943,18 +1057,20 @@ void ElementMap::addChildElements(long masterTag, const std::vectorchildElements.empty()) { - if (expansion.size()) + if (!expansion.empty()) { expansion.push_back(child); + } continue; } auto& indices = child.elementMap->indexedNames[child.indexedName.getType()]; if (indices.children.empty()) { - if (expansion.size()) + if (!expansion.empty()) { expansion.push_back(child); + } continue; } - // Note that it is allow to have both mapped names and child map. We + // Note that it is allowable to have both mapped names and child map. We // may have to split the current child mapping into pieces. int start = child.indexedName.getIndex(); @@ -964,10 +1080,11 @@ void ElementMap::addChildElements(long masterTag, const std::vectorsecond; int istart = grandchild.indexedName.getIndex() + grandchild.offset; int iend = istart + grandchild.count; - if (end <= istart) + if (end <= istart) { break; + } if (istart >= end) { - if (expansion.size()) { + if (!expansion.empty()) { expansion.push_back(child); expansion.back().indexedName.setIndex(start); expansion.back().count = end - start; @@ -975,7 +1092,8 @@ void ElementMap::addChildElements(long masterTag, const std::vector end) + if (iend > end) { iend = end; + } entry->indexedName.setIndex(istart - grandchild.offset); entry->count = iend - istart; entry->offset += grandchild.offset; entry->elementMap = grandchild.elementMap; entry->sids += grandchild.sids; - if (grandchild.postfix.size()) { - if (entry->postfix.size() + if (grandchild.postfix.size() != 0) { + if ((entry->postfix.size() != 0) && !entry->postfix.startsWith(ELEMENT_MAP_PREFIX)) { - entry->postfix = - grandchild.postfix + ELEMENT_MAP_PREFIX + entry->postfix; + entry->postfix = grandchild.postfix + ELEMENT_MAP_PREFIX + entry->postfix; } - else + else { entry->postfix = grandchild.postfix + entry->postfix; + } } start = iend; - if (start >= end) + if (start >= end) { break; + } } - if (expansion.size() && start < end) { + if (!expansion.empty() && start < end) { expansion.push_back(child); expansion.back().indexedName.setIndex(start); expansion.back().count = end - start; } } - for (auto& child : expansion.size() ? expansion : children) { - if (!child.indexedName || !child.count) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) - FC_ERR("invalid mapped child element"); + for (auto& child : expansion.empty() ? children : expansion) { + if (!child.indexedName || (child.count == 0)) { + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { + FC_ERR("invalid mapped child element");// NOLINT + } continue; } @@ -1032,7 +1154,8 @@ void ElementMap::addChildElements(long masterTag, const std::vector= 5 - if (child.count >= 5 || !child.elementMap) { + const int threshold {5}; + if (child.count >= threshold || !child.elementMap) { encodeElementName(child.indexedName[0], tmp, ss, @@ -1061,9 +1184,10 @@ void ElementMap::addChildElements(long masterTag, const std::vectorfind(childIdx, &sids); if (!name) { - if (!child.tag || child.tag == masterTag) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) - FC_WARN("unmapped element"); + if ((child.tag == 0) || child.tag == masterTag) { + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { + FC_WARN("unmapped element");// NOLINT + } continue; } name = MappedName(childIdx); @@ -1079,7 +1203,7 @@ void ElementMap::addChildElements(long masterTag, const std::vectorindex != 1) { // There is some ambiguity in child mapping. We need some // additional postfix for disambiguation. NOTE: We are not - // using ComplexGeoData::indexPostfix() so as to not confuse + // using ComplexGeoData::indexPostfix() so we don't confuse // other code that actually uses this postfix for indexing // purposes. Here, we just need some postfix for // disambiguation. We don't need to extract the index. @@ -1098,7 +1222,7 @@ void ElementMap::addChildElements(long masterTag, const std::vectorchildMap) { - FC_ERR("duplicate mapped child element"); + FC_ERR("duplicate mapped child element");// NOLINT continue; } } @@ -1107,9 +1231,10 @@ void ElementMap::addChildElements(long masterTag, const std::vectorchildMap) + if (!entry->childMap) { this->childElements.remove(tmp.toBytes()); - FC_ERR("duplicate mapped child element"); + } + FC_ERR("duplicate mapped child element");// NOLINT continue; } @@ -1124,8 +1249,9 @@ std::vector ElementMap::getChildElements() cons { std::vector res; res.reserve(this->childElements.size()); - for (auto& v : this->childElements) - res.push_back(*v.childMap); + for (auto& childElement : this->childElements) { + res.push_back(*childElement.childMap); + } return res; } @@ -1133,19 +1259,22 @@ std::vector ElementMap::getAll() const { std::vector ret; ret.reserve(size()); - for (auto& v : this->mappedNames) - ret.emplace_back(v.first, v.second); - for (auto& v : this->childElements) { - auto& child = *v.childMap; + for (auto& mappedName : this->mappedNames) { + ret.emplace_back(mappedName.first, mappedName.second); + } + for (auto& childElement : this->childElements) { + auto& child = *childElement.childMap; IndexedName idx(child.indexedName); idx.setIndex(idx.getIndex() + child.offset); IndexedName childIdx(child.indexedName); for (int i = 0; i < child.count; ++i, ++idx, ++childIdx) { MappedName name; - if (child.elementMap) + if (child.elementMap) { name = child.elementMap->find(childIdx); - else + } + else { name = MappedName(childIdx); + } if (name) { name += child.postfix; ret.emplace_back(name, idx); @@ -1155,58 +1284,62 @@ std::vector ElementMap::getAll() const return ret; } -long ElementMap::getElementHistory(const MappedName & name, - long masterTag, - MappedName *original, - std::vector *history) const +long ElementMap::getElementHistory(const MappedName& name, long masterTag, MappedName* original, + std::vector* history) const { long tag = 0; int len = 0; - int pos = name.findTagInElementName(&tag,&len,nullptr,nullptr,true); - if(pos < 0) { - if(original) + int pos = name.findTagInElementName(&tag, &len, nullptr, nullptr, true); + if (pos < 0) { + if (original) { *original = name; + } return tag; } - if(!original && !history) + if (!original && !history) { return tag; + } MappedName tmp; - MappedName &ret = original?*original:tmp; - if(name.startsWith(ELEMENT_MAP_PREFIX)) { + MappedName& ret = original ? *original : tmp; + if (name.startsWith(ELEMENT_MAP_PREFIX)) { unsigned offset = ELEMENT_MAP_PREFIX_SIZE; - ret = MappedName::fromRawData(name, offset); - } else + ret = MappedName::fromRawData(name, static_cast(offset)); + } + else { ret = name; + } - while(1) { - if(!len || len>pos) { - FC_WARN("invalid name length " << name); + while (true) { + if ((len == 0) || len > pos) { + FC_WARN("invalid name length " << name);// NOLINT return 0; } - bool dehashed = false; + bool deHashed = false; if (ret.startsWith(MAPPED_CHILD_ELEMENTS_PREFIX, len)) { int offset = (int)POSTFIX_TAG_SIZE; - MappedName tmp = MappedName::fromRawData(ret, len+offset, pos-len-offset); - MappedName postfix = dehashElementName(tmp); - if (postfix != tmp) { - dehashed = true; + MappedName tmp2 = MappedName::fromRawData(ret, len + offset, pos - len - offset); + MappedName postfix = dehashElementName(tmp2); + if (postfix != tmp2) { + deHashed = true; ret = MappedName::fromRawData(ret, 0, len) + postfix; } } - if (!dehashed) + if (!deHashed) { ret = dehashElementName(MappedName::fromRawData(ret, 0, len)); + } long tag2 = 0; - pos = ret.findTagInElementName(&tag2,&len,nullptr,nullptr,true); - if(pos < 0 || (tag2!=tag && tag2!=-tag && tag!=masterTag && -tag!=masterTag)) + pos = ret.findTagInElementName(&tag2, &len, nullptr, nullptr, true); + if (pos < 0 || (tag2 != tag && tag2 != -tag && tag != masterTag && -tag != masterTag)) { return tag; + } tag = tag2; - if(history) + if (history) { history->push_back(ret.copy()); + } } } - }// Namespace Data diff --git a/src/App/ElementMap.h b/src/App/ElementMap.h index aea3e9ff22..fd598fecd1 100644 --- a/src/App/ElementMap.h +++ b/src/App/ElementMap.h @@ -63,27 +63,27 @@ public: ElementMap(); /** Ensures that naming is properly assigned. It then marks as "used" all the StringID - * that are used to make up this particular map and are stored in the hasher passed + * that are used to make up this particular map and are stored in the hasherRef passed * as a parameter. Finally do this recursively for all childEelementMaps as well. * - * @param hasher where all the StringID needed to build the map are stored. + * @param hasherRef where all the StringID needed to build the map are stored. */ // FIXME this should be made part of \c save, to achieve symmetry with the restore method - void beforeSave(const ::App::StringHasherRef& hasher) const; + void beforeSave(const ::App::StringHasherRef& hasherRef) const; /** Serialize this map. Calls \c collectChildMaps to get \c childMapSet and * \c postfixMap, then calls the other (private) save function with those parameters. - * @param s: serialized stream + * @param stream: serialized stream */ - void save(std::ostream& s) const; + void save(std::ostream& stream) const; - /** Deserialize and restore this map. This function restores \c childMaps and + /** Deserialize and restore this map. This function restores \c childMaps and * \c postfixes from the stream, then calls the other (private) restore function with those * parameters. - * @param hasher: where all the StringIDs are stored - * @param s: stream to deserialize + * @param hasherRef: where all the StringIDs are stored + * @param stream: stream to deserialize */ - ElementMapPtr restore(::App::StringHasherRef hasher, std::istream& s); + ElementMapPtr restore(::App::StringHasherRef hasherRef, std::istream& stream); /** Add a sub-element name mapping. @@ -187,20 +187,20 @@ public: private: /** Serialize this map - * @param s: serialized stream + * @param stream: serialized stream * @param childMapSet: where all child element maps are stored * @param postfixMap. where all postfixes are stored */ - void save(std::ostream& s, int index, const std::map& childMapSet, + void save(std::ostream& stream, int index, const std::map& childMapSet, const std::map& postfixMap) const; /** Deserialize and restore this map. - * @param hasher: where all the StringIDs are stored - * @param s: stream to deserialize + * @param hasherRef: where all the StringIDs are stored + * @param stream: stream to deserialize * @param childMaps: where all child element maps are stored * @param postfixes. where all postfixes are stored */ - ElementMapPtr restore(::App::StringHasherRef hasher, std::istream& s, + ElementMapPtr restore(::App::StringHasherRef hasherRef, std::istream& stream, std::vector& childMaps, const std::vector& postfixes); @@ -224,9 +224,9 @@ private: /* Note: the original proc passed `ComplexGeoData& master` for getting the `Tag`, * now it just passes `long masterTag`.*/ - virtual MappedName renameDuplicateElement(int index, const IndexedName& element, - const IndexedName& element2, const MappedName& name, - ElementIDRefs& sids, long masterTag); + MappedName renameDuplicateElement(int index, const IndexedName& element, + const IndexedName& element2, const MappedName& name, + ElementIDRefs& sids, long masterTag) const; /** Convenience method to hash the main element name * diff --git a/src/App/MappedName.h b/src/App/MappedName.h index 54a58ea19f..9921e4280e 100644 --- a/src/App/MappedName.h +++ b/src/App/MappedName.h @@ -34,10 +34,11 @@ #include #include #include +#include +#include "ElementNamingUtils.h" #include "IndexedName.h" #include "StringHasher.h" -#include "ElementNamingUtils.h" namespace Data @@ -92,8 +93,9 @@ public: /// is appended as text to the MappedName. In that case the memory is *not* shared between the /// original IndexedName and the MappedName. explicit MappedName(const IndexedName& element) - : data(QByteArray::fromRawData(element.getType(), qstrlen(element.getType()))), - raw(true) + : data(QByteArray::fromRawData(element.getType(), + static_cast(qstrlen(element.getType())))) + , raw(true) { if (element.getIndex() > 0) { this->data += QByteArray::number(element.getIndex()); @@ -101,8 +103,8 @@ public: } } - explicit MappedName(const App::StringIDRef & sid) - :raw(false) + explicit MappedName(const App::StringIDRef& sid) + : raw(false) { sid.toBytes(this->data); } @@ -130,16 +132,16 @@ public: /// \param other The mapped name to copy. Its data and postfix become the new MappedName's data /// \param postfix The postfix for the new MappedName MappedName(const MappedName& other, const char* postfix) - : data(other.data + other.postfix), - postfix(postfix), - raw(false) + : data(other.data + other.postfix) + , postfix(postfix) + , raw(false) {} /// Move constructor MappedName(MappedName&& other) noexcept - : data(std::move(other.data)), - postfix(std::move(other.postfix)), - raw(other.raw) + : data(std::move(other.data)) + , postfix(std::move(other.postfix)) + , raw(other.raw) {} ~MappedName() = default; @@ -899,7 +901,7 @@ public: /// /// \param tag: optional pointer to receive the extracted tag /// \param len: optional pointer to receive the length field after the tag field. - /// This gives the length of the previous hashsed element name starting + /// This gives the length of the previous hashed element name starting /// from the beginning of the give element name. /// \param postfix: optional pointer to receive the postfix starting at the found tag field. /// \param type: optional pointer to receive the element type character @@ -908,8 +910,9 @@ public: /// \param recursive: recursively find the last non-zero tag /// /// \return Return the end position of the tag field, or return -1 if not found. - int findTagInElementName(long* tag = 0, int* len = 0, const char* postfix = 0, char* type = 0, - bool negative = false, bool recursive = true) const; + int findTagInElementName(long* tag = nullptr, int* len = nullptr, const char* postfix = nullptr, + char* type = nullptr, bool negative = false, + bool recursive = true) const; /// Get a hash for this MappedName std::size_t hash() const @@ -924,7 +927,7 @@ private: }; -typedef QVector<::App::StringIDRef> ElementIDRefs; +using ElementIDRefs = QVector<::App::StringIDRef>; struct MappedNameRef { @@ -934,25 +937,34 @@ struct MappedNameRef MappedNameRef() = default; - MappedNameRef(const MappedName& name, const ElementIDRefs& sids = ElementIDRefs()) - : name(name), - sids(sids) + ~MappedNameRef() = default; + + MappedNameRef(MappedName name, ElementIDRefs sids = ElementIDRefs()) + : name(std::move(name)) + , sids(std::move(sids)) { compact(); } MappedNameRef(const MappedNameRef& other) - : name(other.name), - sids(other.sids) + : name(other.name) + , sids(other.sids) {} - MappedNameRef(MappedNameRef&& other) - : name(std::move(other.name)), - sids(std::move(other.sids)), - next(std::move(other.next)) + MappedNameRef(MappedNameRef&& other) noexcept + : name(std::move(other.name)) + , sids(std::move(other.sids)) + , next(std::move(other.next)) {} - MappedNameRef& operator=(MappedNameRef&& other) + MappedNameRef& operator=(const MappedNameRef& other) noexcept + { + name = other.name; + sids = other.sids; + return *this; + } + + MappedNameRef& operator=(MappedNameRef&& other) noexcept { name = std::move(other.name); sids = std::move(other.sids); @@ -965,22 +977,24 @@ struct MappedNameRef return !name.empty(); } - void append(const MappedName& name, const ElementIDRefs sids = ElementIDRefs()) + void append(const MappedName& _name, const ElementIDRefs _sids = ElementIDRefs()) { - if (!name) + if (!_name) { return; + } if (!this->name) { - this->name = name; - this->sids = sids; + this->name = _name; + this->sids = _sids; compact(); return; } - std::unique_ptr n(new MappedNameRef(name, sids)); - if (!this->next) - this->next = std::move(n); + std::unique_ptr mappedName(new MappedNameRef(_name, _sids)); + if (!this->next) { + this->next = std::move(mappedName); + } else { - this->next.swap(n); - this->next->next = std::move(n); + this->next.swap(mappedName); + this->next->next = std::move(mappedName); } } @@ -992,9 +1006,9 @@ struct MappedNameRef } } - bool erase(const MappedName& name) + bool erase(const MappedName& _name) { - if (this->name == name) { + if (this->name == _name) { this->name.clear(); this->sids.clear(); if (this->next) { @@ -1007,11 +1021,11 @@ struct MappedNameRef return true; } - for (std::unique_ptr* p = &this->next; *p; p = &(*p)->next) { - if ((*p)->name == name) { + for (std::unique_ptr* ptr = &this->next; *ptr; ptr = &(*ptr)->next) { + if ((*ptr)->name == _name) { std::unique_ptr tmp; - tmp.swap(*p); - *p = std::move(tmp->next); + tmp.swap(*ptr); + *ptr = std::move(tmp->next); return true; } } @@ -1033,4 +1047,4 @@ struct MappedNameRef }// namespace Data -#endif// APP_MAPPED_NAME_H \ No newline at end of file +#endif// APP_MAPPED_NAME_H diff --git a/src/App/StringHasher.h b/src/App/StringHasher.h index 7c8f4f5dcf..e56ba9d9ac 100644 --- a/src/App/StringHasher.h +++ b/src/App/StringHasher.h @@ -173,7 +173,7 @@ public: } /// Returns the data (prefix) - QByteArray data() const + const QByteArray& data() const { return _data; } @@ -556,6 +556,7 @@ public: void toBytes(QByteArray& bytes) const { + // TODO: return the QByteArray instead of passing in by reference if (_sid) { bytes = _sid->dataToBytes(_index); } diff --git a/tests/src/App/CMakeLists.txt b/tests/src/App/CMakeLists.txt index e14c01958f..8a98989dc9 100644 --- a/tests/src/App/CMakeLists.txt +++ b/tests/src/App/CMakeLists.txt @@ -3,6 +3,7 @@ target_sources( PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/Application.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Branding.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/ComplexGeoData.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Expression.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ElementMap.cpp ${CMAKE_CURRENT_SOURCE_DIR}/IndexedName.cpp diff --git a/tests/src/App/ComplexGeoData.cpp b/tests/src/App/ComplexGeoData.cpp new file mode 100644 index 0000000000..2447c6209c --- /dev/null +++ b/tests/src/App/ComplexGeoData.cpp @@ -0,0 +1,417 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "gtest/gtest.h" + +#include +#include + +#include +#include +#include + + +class ConcreteComplexGeoDataForTesting: public Data::ComplexGeoData +{ +public: + std::vector getElementTypes() const override + { + return {"EDGE"}; + } + + unsigned long countSubElements(const char* Type) const override + { + (void)Type; + return 0; + } + + Data::Segment* getSubElement(const char* Type, unsigned long number) const override + { + (void)number; + (void)Type; + return nullptr; + } + + unsigned int getMemSize() const override + { + return 0; + } + + void Save(Base::Writer& writer) const override + { + (void)writer; + } + + void Restore(Base::XMLReader& reader) override + { + (void)reader; + } + + void setTransform(const Base::Matrix4D& rclTrf) override + { + (void)rclTrf; + } + + Base::Matrix4D getTransform() const override + { + return {}; + } + + void transformGeometry(const Base::Matrix4D& rclMat) override + { + (void)rclMat; + } + + Base::BoundBox3d getBoundBox() const override + { + return Base::BoundBox3d(); + } +}; + +class ComplexGeoDataTest: public ::testing::Test +{ +protected: + static void SetUpTestSuite() + { + if (App::Application::GetARGC() == 0) { + constexpr int argc = 1; + std::array argv {"FreeCAD"}; + App::Application::Config()["ExeName"] = "FreeCAD"; + App::Application::init(argc, argv.data()); + } + } + + void SetUp() override + { + _docName = App::GetApplication().getUniqueDocumentName("test"); + App::GetApplication().newDocument(_docName.c_str(), "testUser"); + } + + void TearDown() override + { + App::GetApplication().closeDocument(_docName.c_str()); + } + + ConcreteComplexGeoDataForTesting& cgd() + { + return _complexGeoData; + } + + std::tuple createMappedName(const std::string& name) + { + auto elementMap = std::make_shared(); + cgd().resetElementMap(elementMap); + auto mappedName = Data::MappedName(name.c_str()); + auto indexedName = Data::IndexedName(_complexGeoData.getElementTypes().front(), 1); + elementMap->setElementName(indexedName, mappedName, 0); + return std::make_tuple(indexedName, mappedName); + } + +private: + ConcreteComplexGeoDataForTesting _complexGeoData; + std::string _docName; +}; + +// NOLINTBEGIN(readability-magic-numbers) + +TEST_F(ComplexGeoDataTest, getIndexedNameNoName)// NOLINT +{ + // Arrange & Act + auto result = cgd().getIndexedName(Data::MappedName()); + + // Assert + EXPECT_FALSE(result); +} + +TEST_F(ComplexGeoDataTest, getIndexedNameNoElementMap)// NOLINT +{ + // Arrange & Act + auto result = cgd().getIndexedName(Data::MappedName("TestName")); + + // Assert + EXPECT_TRUE(result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameNoElement)// NOLINT +{ + // Arrange & Act + auto result = cgd().getMappedName(Data::IndexedName {}); + + // Assert + EXPECT_FALSE(result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameDisallowUnmappedNoMap)// NOLINT +{ + // Arrange & Act + auto result = cgd().getMappedName(Data::IndexedName {"TestName"}, false); + + // Assert + EXPECT_FALSE(result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameDisallowUnmappedWithMap)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + cgd().resetElementMap(elementMap); + auto mappedName = Data::MappedName("TestMappedName"); + auto indexedName = Data::IndexedName("EDGE", 1); + elementMap->setElementName(indexedName, mappedName, 0); + + // Act + auto result = cgd().getMappedName(indexedName, false); + + // Assert + EXPECT_TRUE(result); + EXPECT_EQ(mappedName, result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameDisallowUnmappedMissingFromMap)// NOLINT +{ + // Arrange + auto mappedName = Data::MappedName("NotTheTestName"); + cgd().getIndexedName(mappedName);// Put it in the map + + // Act + auto result = cgd().getMappedName(Data::IndexedName {"TestName"}, false); + + // Assert + EXPECT_FALSE(result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameAllowUnmapped)// NOLINT +{ + // Arrange & Act + auto result = cgd().getMappedName(Data::IndexedName {"TestName"}, true); + + // Assert + EXPECT_TRUE(result); +} + +TEST_F(ComplexGeoDataTest, getElementNameGivenIndexedName)// NOLINT +{ + // Arrange + const char* name {"EDGE123"}; + auto indexedName = cgd().getIndexedName(Data::MappedName(name)); + + // Act + auto result = cgd().getElementName(name); + + // Assert + EXPECT_EQ(result.index, indexedName); +} + +TEST_F(ComplexGeoDataTest, getElementNameGivenMappedName)// NOLINT +{ + // Arrange + const char* name {"EDGE123"}; + auto mappedName = cgd().getMappedName(Data::IndexedName(name)); + + // Act + auto result = cgd().getElementName(name); + + // Assert + EXPECT_EQ(result.name, mappedName); +} + +TEST_F(ComplexGeoDataTest, getElementMappedNamesNoMapNoUnmapped)// NOLINT +{ + // Arrange + // Do not create an element map + auto indexedName = Data::IndexedName("EDGE1"); + + // Act + auto result = cgd().getElementMappedNames(indexedName, false); + + // Assert + EXPECT_TRUE(result.empty()); +} + +TEST_F(ComplexGeoDataTest, getElementMappedNamesWithMapNoUnmapped)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + cgd().resetElementMap(elementMap); + auto mappedName = Data::MappedName("TestMappedName"); + auto indexedName = Data::IndexedName("EDGE", 1); + elementMap->setElementName(indexedName, mappedName, 0); + + // Act + auto result = cgd().getElementMappedNames(indexedName, false); + + // Assert + EXPECT_FALSE(result.empty()); + EXPECT_EQ(result[0].first, mappedName); +} + +TEST_F(ComplexGeoDataTest, getElementMappedNamesNoMapWithUnmapped)// NOLINT +{ + // Do not create an element map + auto indexedName = Data::IndexedName("EDGE1"); + + // Act + auto result = cgd().getElementMappedNames(indexedName, true); + + // Assert + EXPECT_FALSE(result.empty()); +} + + +TEST_F(ComplexGeoDataTest, getElementMappedNamesWithMapWithUnmapped)// NOLINT +{ + // Arrange + Data::MappedName mappedName; + Data::IndexedName indexedName; + std::tie(indexedName, mappedName) = createMappedName("TestMappedName"); + auto unmappedName = Data::IndexedName("EDGE", 2); + + // Act + auto result = cgd().getElementMappedNames(unmappedName, true); + + // Assert + EXPECT_FALSE(result.empty()); + EXPECT_NE(result[0].first, mappedName); +} + +TEST_F(ComplexGeoDataTest, elementTypeValidIndexName)// NOLINT +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE", 1); + + // Act + char elementType = cgd().elementType(indexedName); + + // Assert + EXPECT_EQ(elementType, 'E'); +} + +TEST_F(ComplexGeoDataTest, elementTypeInvalidIndexedName)// NOLINT +{ + // Arrange + auto indexedName = Data::IndexedName("INVALID", 1);// Not in the element type list + + // Act + char elementType = cgd().elementType(indexedName); + + // Assert + EXPECT_EQ(elementType, 0); +} + +TEST_F(ComplexGeoDataTest, elementTypeCharEmptyName)// NOLINT +{ + // Arrange & Act + char elementType = cgd().elementType(nullptr); + + // Assert + EXPECT_EQ(elementType, 0); +} + +TEST_F(ComplexGeoDataTest, elementTypeCharIndexedName)// NOLINT +{ + // Arrange & Act + char elementType = cgd().elementType("EDGE1"); + + // Assert + EXPECT_EQ(elementType, 'E'); +} + +TEST_F(ComplexGeoDataTest, elementTypeCharMappedNameNoPrefix)// NOLINT +{ + // Arrange + int size {0}; + Data::MappedName mappedName; + Data::IndexedName indexedName; + std::tie(indexedName, mappedName) = createMappedName("TestMappedName:;"); + + // Act + char elementType = cgd().elementType(mappedName.toConstString(0, size)); + + // Assert + EXPECT_EQ(elementType, 'E'); +} + +TEST_F(ComplexGeoDataTest, elementTypeCharMappedNameWithPrefix)// NOLINT +{ + // Arrange + int size {0}; + Data::MappedName mappedName; + Data::IndexedName indexedName; + auto name = fmt::format("{}TestMappedElement:;", Data::ELEMENT_MAP_PREFIX); + std::tie(indexedName, mappedName) = createMappedName(name); + + // Act + char elementType = cgd().elementType(mappedName.toConstString(0, size)); + + // Assert + EXPECT_EQ(elementType, 'E'); +} + +TEST_F(ComplexGeoDataTest, resetElementMapNoArgument)// NOLINT +{ + // Arrange & Act + cgd().resetElementMap(); + + // Assert + EXPECT_EQ(cgd().getElementMapSize(), 0); +} + +TEST_F(ComplexGeoDataTest, resetElementMapWithArgument)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + auto mappedName = Data::MappedName("TestMappedName"); + auto indexedName = Data::IndexedName("EDGE", 1); + elementMap->setElementName(indexedName, mappedName, 0); + + // Act + cgd().resetElementMap(elementMap); + + // Assert + EXPECT_EQ(cgd().getElementMapSize(), 1); +} + +TEST_F(ComplexGeoDataTest, setAndGetElementMap)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + cgd().resetElementMap(elementMap); + std::vector vecMappedElements; + auto mappedNameA = Data::MappedName("TestMappedNameA"); + auto indexedNameA = Data::IndexedName("EDGE", 1); + vecMappedElements.emplace_back(indexedNameA, mappedNameA); + auto mappedNameB = Data::MappedName("TestMappedNameB"); + auto indexedNameB = Data::IndexedName("EDGE", 2); + vecMappedElements.emplace_back(indexedNameB, mappedNameB); + + // Act + auto emptyElementMap = cgd().getElementMap(); + cgd().setElementMap(vecMappedElements); + auto resultingElementMap = cgd().getElementMap(); + + // Assert + EXPECT_TRUE(emptyElementMap.empty()); + EXPECT_EQ(resultingElementMap.size(), vecMappedElements.size()); +} + +TEST_F(ComplexGeoDataTest, getElementMapSize)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + auto mappedName = Data::MappedName("TestMappedName"); + auto indexedName = Data::IndexedName("EDGE", 1); + elementMap->setElementName(indexedName, mappedName, 0); + cgd().resetElementMap(elementMap); + + // Act + auto result = cgd().getElementMapSize(); + + // Assert + EXPECT_EQ(result, 1); +} + +TEST_F(ComplexGeoDataTest, flushElementMap)// NOLINT +{ + // Does nothing in the base class +} + +// NOLINTEND(readability-magic-numbers) diff --git a/tests/src/App/ElementMap.cpp b/tests/src/App/ElementMap.cpp index 13819bd723..3e103be85a 100644 --- a/tests/src/App/ElementMap.cpp +++ b/tests/src/App/ElementMap.cpp @@ -45,21 +45,29 @@ class ElementMapTest: public ::testing::Test protected: static void SetUpTestSuite() { - int argc = 1; - char* argv[] = {"FreeCAD"}; - App::Application::Config()["ExeName"] = "FreeCAD"; - App::Application::init(argc, argv); + if (App::Application::GetARGC() == 0) { + int argc = 1; + char* argv[] = {"FreeCAD"}; + App::Application::Config()["ExeName"] = "FreeCAD"; + App::Application::init(argc, argv); + } } void SetUp() override { - App::GetApplication().newDocument("test", "testUser"); + _docName = App::GetApplication().getUniqueDocumentName("test"); + App::GetApplication().newDocument(_docName.c_str(), "testUser"); _sids = &_sid; _hasher = Base::Reference(new App::StringHasher); + ASSERT_EQ(_hasher.getRefCount(), 1); } - // void TearDown() override {} + void TearDown() override + { + App::GetApplication().closeDocument(_docName.c_str()); + } + std::string _docName; Data::ElementIDRefs _sid; QVector* _sids; App::StringHasherRef _hasher; diff --git a/tests/src/App/MappedName.cpp b/tests/src/App/MappedName.cpp index 4b11ec5f6e..77b6b297f0 100644 --- a/tests/src/App/MappedName.cpp +++ b/tests/src/App/MappedName.cpp @@ -225,8 +225,11 @@ TEST(MappedName, fromRawData) TEST(MappedName, fromRawDataQByteArray) { + // Arrange + QByteArray testByteArray("TESTTEST", 10); + // Act - Data::MappedName mappedName = Data::MappedName::fromRawData(QByteArray("TESTTEST", 10)); + Data::MappedName mappedName = Data::MappedName::fromRawData(testByteArray); // Assert EXPECT_EQ(mappedName.isRaw(), true); @@ -239,7 +242,8 @@ TEST(MappedName, fromRawDataQByteArray) TEST(MappedName, fromRawDataCopy) { // Arrange - Data::MappedName temp = Data::MappedName::fromRawData(QByteArray("TESTTEST", 10)); + QByteArray testByteArray("TESTTEST", 10); + Data::MappedName temp = Data::MappedName::fromRawData(testByteArray); temp.append("TESTPOSTFIX"); temp.compact();// Always call compact before accessing data! @@ -257,7 +261,8 @@ TEST(MappedName, fromRawDataCopy) TEST(MappedName, fromRawDataCopyStartposAndSize) { // Arrange - Data::MappedName temp = Data::MappedName::fromRawData(QByteArray("TESTTEST", 8)); + QByteArray testByteArray("TESTTEST", 8); + Data::MappedName temp = Data::MappedName::fromRawData(testByteArray); temp.append("ABCDEFGHIJKLM");// postfix temp.compact(); // Always call compact before accessing data! @@ -373,11 +378,12 @@ TEST(MappedName, additionOperators) { // Arrange Data::MappedName mappedName(Data::MappedName("TEST"), "POSTFIXTEST"); + QByteArray post3("POST3"); // Act mappedName += "POST1"; mappedName += std::string("POST2"); - mappedName += QByteArray("POST3"); + mappedName += post3; mappedName += Data::MappedName("POST4"); // Assert @@ -389,12 +395,13 @@ TEST(MappedName, additionOperators) // Arrange mappedName = Data::MappedName(Data::MappedName("TEST"), "POSTFIXTEST"); + QByteArray post8("POST8"); // Act mappedName = mappedName + Data::MappedName("POST5"); mappedName = mappedName + "POST6"; mappedName = mappedName + std::string("POST7"); - mappedName = mappedName + QByteArray("POST8"); + mappedName = mappedName + post8; // Assert EXPECT_EQ(mappedName.isRaw(), false);