diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 338dedbca8..62eafd2a41 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -111,41 +111,30 @@ static bool globalIsRelabeling; DocumentP::DocumentP() { - Hasher = new StringHasher; - static std::random_device _RD; - static std::mt19937 _RGEN(_RD()); - static std::uniform_int_distribution<> _RDIST(0, 5000); + static std::random_device rd; + static std::mt19937 rgen(rd()); + static std::uniform_int_distribution<> rdist(0, 5000); // Set some random offset to reduce likelihood of ID collision when // copying shape from other document. It is probably better to randomize // on each object ID. - lastObjectId = _RDIST(_RGEN); - activeObject = nullptr; - activeUndoTransaction = nullptr; - iTransactionMode = 0; - rollback = false; - undoing = false; - committing = false; - opentransaction = false; + lastObjectId = rdist(rgen); StatusBits.set((size_t)Document::Closable, true); StatusBits.set((size_t)Document::KeepTrailingDigits, true); StatusBits.set((size_t)Document::Restoring, false); - iUndoMode = 0; - UndoMemSize = 0; - UndoMaxStackSize = 20; } } // namespace App PROPERTY_SOURCE(App::Document, App::PropertyContainer) -bool Document::testStatus(Status pos) const +bool Document::testStatus(const Status pos) const { - return d->StatusBits.test((size_t)pos); + return d->StatusBits.test(static_cast(pos)); } -void Document::setStatus(Status pos, bool on) +void Document::setStatus(const Status pos, const bool on) // NOLINT { - d->StatusBits.set((size_t)pos, on); + d->StatusBits.set(static_cast(pos), on); } // bool _has_cycle_dfs(const DependencyList & g, vertex_t u, default_color_type * color) @@ -164,22 +153,14 @@ void Document::setStatus(Status pos, bool on) bool Document::checkOnCycle() { -#if 0 - std::vector < default_color_type > color(num_vertices(_DepList), white_color); - graph_traits < DependencyList >::vertex_iterator vi, vi_end; - for (tie(vi, vi_end) = vertices(_DepList); vi != vi_end; ++vi) - if (color[*vi] == white_color) - if (_has_cycle_dfs(_DepList, *vi, &color[0])) - return true; -#endif return false; } -bool Document::undo(int id) +bool Document::undo(const int id) { - if (d->iUndoMode) { - if (id) { - auto it = mUndoMap.find(id); + if (d->iUndoMode != 0) { + if (id != 0) { + const auto it = mUndoMap.find(id); if (it == mUndoMap.end()) { return false; } @@ -215,7 +196,7 @@ bool Document::undo(int id) mUndoTransactions.pop_back(); } - for (auto& obj : d->objectArray) { + for (const auto& obj : d->objectArray) { if (obj->testStatus(ObjectStatus::PendingTransactionUpdate)) { obj->onUndoRedoFinished(); obj->setStatus(ObjectStatus::PendingTransactionUpdate, false); @@ -230,11 +211,11 @@ bool Document::undo(int id) return false; } -bool Document::redo(int id) +bool Document::redo(const int id) { - if (d->iUndoMode) { - if (id) { - auto it = mRedoMap.find(id); + if (d->iUndoMode != 0) { + if (id != 0) { + const auto it = mRedoMap.find(id); if (it == mRedoMap.end()) { return false; } @@ -267,7 +248,7 @@ bool Document::redo(int id) mRedoTransactions.pop_back(); } - for (auto& obj : d->objectArray) { + for (const auto& obj : d->objectArray) { if (obj->testStatus(ObjectStatus::PendingTransactionUpdate)) { obj->onUndoRedoFinished(); obj->setStatus(ObjectStatus::PendingTransactionUpdate, false); @@ -281,12 +262,13 @@ bool Document::redo(int id) return false; } -void Document::addOrRemovePropertyOfObject(TransactionalObject* obj, Property* prop, bool add) +void Document::addOrRemovePropertyOfObject(TransactionalObject* obj, + const Property* prop, const bool add) { if (!prop || !obj || !obj->isAttachedToDocument()) { return; } - if (d->iUndoMode && !isPerformingTransaction() && !d->activeUndoTransaction) { + if ((d->iUndoMode != 0) && !isPerformingTransaction() && !d->activeUndoTransaction) { if (!testStatus(Restoring) || testStatus(Importing)) { int tid = 0; const char* name = GetApplication().getActiveTransaction(&tid); @@ -311,10 +293,10 @@ std::vector Document::getAvailableUndoNames() const if (d->activeUndoTransaction) { vList.push_back(d->activeUndoTransaction->Name); } - for (std::list::const_reverse_iterator It = mUndoTransactions.rbegin(); + for (auto It = mUndoTransactions.rbegin(); It != mUndoTransactions.rend(); ++It) { - vList.push_back((**It).Name); + vList.push_back((*It)->Name); } return vList; } @@ -322,15 +304,15 @@ std::vector Document::getAvailableUndoNames() const std::vector Document::getAvailableRedoNames() const { std::vector vList; - for (std::list::const_reverse_iterator It = mRedoTransactions.rbegin(); + for (auto It = mRedoTransactions.rbegin(); It != mRedoTransactions.rend(); ++It) { - vList.push_back((**It).Name); + vList.push_back((*It)->Name); } return vList; } -void Document::openTransaction(const char* name) +void Document::openTransaction(const char* name) // NOLINT { if (isPerformingTransaction() || d->committing) { if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { @@ -351,7 +333,7 @@ int Document::_openTransaction(const char* name, int id) return 0; } - if (d->iUndoMode) { + if (d->iUndoMode != 0) { // Avoid recursive calls that is possible while // clearing the redo transactions and will cause // a double deletion of some transaction and thus @@ -361,7 +343,7 @@ int Document::_openTransaction(const char* name, int id) } Base::FlagToggler<> flag(d->opentransaction); - if (id && mUndoMap.find(id) != mUndoMap.end()) { + if ((id != 0) && mUndoMap.find(id) != mUndoMap.end()) { throw Base::RuntimeError("invalid transaction id"); } if (d->activeUndoTransaction) { @@ -392,7 +374,7 @@ int Document::_openTransaction(const char* name, int id) return 0; } -void Document::renameTransaction(const char* name, int id) +void Document::renameTransaction(const char* name, const int id) const { if (name && d->activeUndoTransaction && d->activeUndoTransaction->getID() == id) { if (boost::starts_with(d->activeUndoTransaction->Name, "-> ")) { @@ -408,7 +390,7 @@ void Document::renameTransaction(const char* name, int id) void Document::_checkTransaction(DocumentObject* pcDelObj, const Property* What, int line) { // if the undo is active but no transaction open, open one! - if (d->iUndoMode && !isPerformingTransaction()) { + if ((d->iUndoMode != 0) && !isPerformingTransaction()) { if (!d->activeUndoTransaction) { if (!testStatus(Restoring) || testStatus(Importing)) { int tid = 0; @@ -464,7 +446,7 @@ void Document::_clearRedos() } } -void Document::commitTransaction() +void Document::commitTransaction() // NOLINT { if (isPerformingTransaction() || d->committing) { if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { @@ -478,7 +460,7 @@ void Document::commitTransaction() } } -void Document::_commitTransaction(bool notify) +void Document::_commitTransaction(const bool notify) { if (isPerformingTransaction()) { if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { @@ -486,7 +468,7 @@ void Document::_commitTransaction(bool notify) } return; } - else if (d->committing) { + if (d->committing) { // for a recursive call return without printing a warning return; } @@ -494,7 +476,7 @@ void Document::_commitTransaction(bool notify) if (d->activeUndoTransaction) { Base::FlagToggler<> flag(d->committing); Application::TransactionSignaller signaller(false, true); - int id = d->activeUndoTransaction->getID(); + const int id = d->activeUndoTransaction->getID(); mUndoTransactions.push_back(d->activeUndoTransaction); d->activeUndoTransaction = nullptr; // check the stack for the limits @@ -512,7 +494,7 @@ void Document::_commitTransaction(bool notify) } } -void Document::abortTransaction() +void Document::abortTransaction() const { if (isPerformingTransaction() || d->committing) { if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { @@ -550,15 +532,10 @@ void Document::_abortTransaction() bool Document::hasPendingTransaction() const { - if (d->activeUndoTransaction) { - return true; - } - else { - return false; - } + return d->activeUndoTransaction != nullptr; } -int Document::getTransactionID(bool undo, unsigned pos) const +int Document::getTransactionID(const bool undo, unsigned pos) const { if (undo) { if (d->activeUndoTransaction) { @@ -571,22 +548,20 @@ int Document::getTransactionID(bool undo, unsigned pos) const return 0; } auto rit = mUndoTransactions.rbegin(); - for (; pos; ++rit, --pos) { - continue; - } + for (; pos != 0U; ++rit, --pos) {} return (*rit)->getID(); } if (pos >= mRedoTransactions.size()) { return 0; } auto rit = mRedoTransactions.rbegin(); - for (; pos; ++rit, --pos) {} + for (; pos != 0U; ++rit, --pos) {} return (*rit)->getID(); } bool Document::isTransactionEmpty() const { - if (d->activeUndoTransaction) { + return !d->activeUndoTransaction; // Transactions are now only created when there are actual changes. // Empty transaction is now significant for marking external changes. It // is used to match ID with transactions in external documents and @@ -594,13 +569,9 @@ bool Document::isTransactionEmpty() const // return d->activeUndoTransaction->isEmpty(); - return false; - } - - return true; } -void Document::clearDocument() +void Document::clearDocument() // NOLINT { d->activeObject = nullptr; @@ -655,10 +626,10 @@ void Document::clearUndos() _clearRedos(); } -int Document::getAvailableUndos(int id) const +int Document::getAvailableUndos(const int id) const { - if (id) { - auto it = mUndoMap.find(id); + if (id != 0) { + const auto it = mUndoMap.find(id); if (it == mUndoMap.end()) { return 0; } @@ -679,15 +650,13 @@ int Document::getAvailableUndos(int id) const if (d->activeUndoTransaction) { return static_cast(mUndoTransactions.size() + 1); } - else { - return static_cast(mUndoTransactions.size()); - } + return static_cast(mUndoTransactions.size()); } -int Document::getAvailableRedos(int id) const +int Document::getAvailableRedos(const int id) const { - if (id) { - auto it = mRedoMap.find(id); + if (id != 0) { + const auto it = mRedoMap.find(id); if (it == mRedoMap.end()) { return 0; } @@ -695,15 +664,15 @@ int Document::getAvailableRedos(int id) const for (auto rit = mRedoTransactions.rbegin(); *rit != it->second; ++rit) { ++i; } - assert(i < (int)mRedoTransactions.size()); + assert(i < static_cast(mRedoTransactions.size())); return i + 1; } return static_cast(mRedoTransactions.size()); } -void Document::setUndoMode(int iMode) +void Document::setUndoMode(const int iMode) { - if (d->iUndoMode && !iMode) { + if ((d->iUndoMode != 0) && (iMode == 0)) { clearUndos(); } @@ -720,12 +689,12 @@ unsigned int Document::getUndoMemSize() const return d->UndoMemSize; } -void Document::setUndoLimit(unsigned int UndoMemSize) +void Document::setUndoLimit(const unsigned int UndoMemSize) // NOLINT { d->UndoMemSize = UndoMemSize; } -void Document::setMaxUndoStackSize(unsigned int UndoMaxStackSize) +void Document::setMaxUndoStackSize(const unsigned int UndoMaxStackSize) // NOLINT { d->UndoMaxStackSize = UndoMaxStackSize; } @@ -750,10 +719,10 @@ void Document::onChanged(const Property* prop) // the Name property is a label for display purposes if (prop == &Label) { Base::FlagToggler<> flag(globalIsRelabeling); - App::GetApplication().signalRelabelDocument(*this); + GetApplication().signalRelabelDocument(*this); } else if (prop == &ShowHidden) { - App::GetApplication().signalShowHidden(*this); + GetApplication().signalShowHidden(*this); } else if (prop == &Uid) { std::string new_dir = @@ -807,8 +776,8 @@ void Document::onChanged(const Property* prop) void Document::onBeforeChangeProperty(const TransactionalObject* Who, const Property* What) { - if (Who->isDerivedFrom()) { - signalBeforeChangeObject(*static_cast(Who), *What); + if (Who->isDerivedFrom()) { + signalBeforeChangeObject(*static_cast(Who), *What); } if (!d->rollback && !globalIsRelabeling) { _checkTransaction(nullptr, What, __LINE__); @@ -823,7 +792,7 @@ void Document::onChangedProperty(const DocumentObject* Who, const Property* What signalChangedObject(*Who, *What); } -void Document::setTransactionMode(int iMode) +void Document::setTransactionMode(const int iMode) // NOLINT { d->iTransactionMode = iMode; } @@ -832,7 +801,7 @@ void Document::setTransactionMode(int iMode) // constructor //-------------------------------------------------------------------------- Document::Document(const char* documentName) - : myName(documentName) + : d(new DocumentP), myName(documentName) { // Remark: In a constructor we should never increment a Python object as we cannot be sure // if the Python interpreter gets a reference of it. E.g. if we increment but Python don't @@ -841,7 +810,6 @@ Document::Document(const char* documentName) // Remark: We force the document Python object to own the DocumentPy instance, thus we don't // have to care about ref counting any more. setAutoCreated(false); - d = new DocumentP; Base::PyGILStateLocker lock; d->DocumentPythonObject = Py::Object(new DocumentPy(this), true); @@ -849,11 +817,11 @@ Document::Document(const char* documentName) Console().log("+App::Document: %p\n", this); #endif std::string CreationDateString = Base::Tools::currentDateTimeString(); - std::string Author = App::GetApplication() + std::string Author = GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetASCII("prefAuthor", ""); std::string AuthorComp = - App::GetApplication() + GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetASCII("prefCompany", ""); ADD_PROPERTY_TYPE(Label, ("Unnamed"), 0, Prop_None, "The name of the document"); @@ -881,7 +849,7 @@ Document::Document(const char* documentName) UnitSystem.setEnums(Base::UnitsApi::getDescriptions()); // Get the preferences/General unit system as the default for a new document ParameterGrp::handle hGrpu = - App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/Units"); + GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/Units"); UnitSystem.setValue(hGrpu->GetInt("UserSchema", 0)); ADD_PROPERTY_TYPE(Comment, (""), 0, Prop_None, "Additional tag to save a comment"); ADD_PROPERTY_TYPE(Meta, (), 0, Prop_None, "Map with additional meta information"); @@ -892,15 +860,14 @@ Document::Document(const char* documentName) ADD_PROPERTY_TYPE(Uid, (id), 0, Prop_ReadOnly, "UUID of the document"); // license stuff - auto paramGrp {App::GetApplication().GetParameterGroupByPath( + auto paramGrp {GetApplication().GetParameterGroupByPath( "User parameter:BaseApp/Preferences/Document")}; auto index = static_cast(paramGrp->GetInt("prefLicenseType", 0)); - const char* name = ""; - const char* url = ""; + auto name = ""; std::string licenseUrl = ""; - if (index >= 0 && index < App::countOfLicenses) { - name = App::licenseItems.at(index).at(App::posnOfFullName); - url = App::licenseItems.at(index).at(App::posnOfUrl); + if (index >= 0 && index < countOfLicenses) { + name = licenseItems.at(index).at(posnOfFullName); + auto url = licenseItems.at(index).at(posnOfUrl); licenseUrl = (paramGrp->GetASCII("prefLicenseUrl", url)); } ADD_PROPERTY_TYPE(License, (name), 0, Prop_None, "License string of the Item"); @@ -963,17 +930,17 @@ Document::~Document() // But we must still invalidate the Python object because it doesn't need to be // destructed right now because the interpreter can own several references to it. Base::PyGILStateLocker lock; - Base::PyObjectBase* doc = static_cast(d->DocumentPythonObject.ptr()); + auto* doc = static_cast(d->DocumentPythonObject.ptr()); // Call before decrementing the reference counter, otherwise a heap error can occur doc->setInvalid(); // remove Transient directory try { - Base::FileInfo TransDir(TransientDir.getValue()); + const Base::FileInfo TransDir(TransientDir.getValue()); TransDir.deleteDirectoryRecursive(); } catch (const Base::Exception& e) { - std::cerr << "Removing transient directory failed: " << e.what() << std::endl; + std::cerr << "Removing transient directory failed: " << e.what() << '\n'; } delete d; } @@ -989,9 +956,9 @@ std::string Document::getTransientDirectoryName(const std::string& uuid, #else hash.addData(QByteArrayView(filename.c_str(), filename.size())); #endif - out << App::Application::getUserCachePath() << App::Application::getExecutableName() << "_Doc_" + out << Application::getUserCachePath() << Application::getExecutableName() << "_Doc_" << uuid << "_" << hash.result().toHex().left(6).constData() << "_" - << App::Application::applicationPid(); + << Application::applicationPid(); return out.str(); } @@ -1005,15 +972,15 @@ void Document::Save(Base::Writer& writer) const addStringHasher(d->Hasher); writer.Stream() << R"(\n"; writer.incInd(); d->Hasher->setPersistenceFileName("StringHasher.Table"); - for (auto o : d->objectArray) { + for (const auto o : d->objectArray) { o->beforeSave(); } beforeSave(); @@ -1026,20 +993,19 @@ void Document::Save(Base::Writer& writer) const // writing the features types writeObjects(d->objectArray, writer); - writer.Stream() << "" << endl; + writer.Stream() << "" << '\n'; } void Document::Restore(Base::XMLReader& reader) { - int i, Cnt; d->hashers.clear(); d->touchedObjs.clear(); addStringHasher(d->Hasher); setStatus(Document::PartialDoc, false); reader.readElement("Document"); - long scheme = reader.getAttribute("SchemaVersion"); - reader.DocumentSchema = scheme; + const long scheme = reader.getAttribute("SchemaVersion"); + reader.DocumentSchema = static_cast(scheme); if (reader.hasAttribute("ProgramVersion")) { reader.ProgramVersion = reader.getAttribute("ProgramVersion"); } @@ -1047,7 +1013,7 @@ void Document::Restore(Base::XMLReader& reader) reader.ProgramVersion = "pre-0.14"; } if (reader.hasAttribute("FileVersion")) { - reader.FileVersion = reader.getAttribute("FileVersion"); + reader.FileVersion = static_cast(reader.getAttribute("FileVersion")); } else { reader.FileVersion = 0; @@ -1067,8 +1033,8 @@ void Document::Restore(Base::XMLReader& reader) // they will be overridden. // Note: This does not affect the internal name of the document in any way // that is kept in Application. - std::string FilePath = FileName.getValue(); - std::string DocLabel = Label.getValue(); + const std::string FilePath = FileName.getValue(); + const std::string DocLabel = Label.getValue(); // read the Document Properties, when reading in Uid the transient directory gets renamed // automatically @@ -1083,8 +1049,7 @@ void Document::Restore(Base::XMLReader& reader) if (scheme == 2) { // read the feature types reader.readElement("Features"); - Cnt = reader.getAttribute("Count"); - for (i = 0; i < Cnt; i++) { + for (auto i = 0; i < reader.getAttribute("Count"); i++) { reader.readElement("Feature"); string type = reader.getAttribute("type"); string name = reader.getAttribute("name"); @@ -1099,8 +1064,7 @@ void Document::Restore(Base::XMLReader& reader) // read the features itself reader.readElement("FeatureData"); - Cnt = reader.getAttribute("Count"); - for (i = 0; i < Cnt; i++) { + for (auto i = 0; i < reader.getAttribute("Count"); i++) { reader.readElement("Feature"); string name = reader.getAttribute("name"); DocumentObject* pObj = getObject(name.c_str()); @@ -1142,14 +1106,14 @@ std::pair Document::addStringHasher(const StringHasherRef& hasher) co return std::make_pair(false, 0); } auto ret = - d->hashers.left.insert(HasherMap::left_map::value_type(hasher, (int)d->hashers.size())); + d->hashers.left.insert(HasherMap::left_map::value_type(hasher, static_cast(d->hashers.size()))); if (ret.second) { hasher->clearMarks(); } return std::make_pair(ret.second, ret.first->second); } -StringHasherRef Document::getStringHasher(int idx) const +StringHasherRef Document::getStringHasher(const int idx) const { StringHasherRef hasher; if (idx < 0) { @@ -1158,7 +1122,7 @@ StringHasherRef Document::getStringHasher(int idx) const } return hasher; } - auto it = d->hashers.right.find(idx); + const auto it = d->hashers.right.find(idx); if (it == d->hashers.right.end()) { hasher = new StringHasher; d->hashers.right.insert(HasherMap::right_map::value_type(idx, hasher)); @@ -1172,25 +1136,25 @@ StringHasherRef Document::getStringHasher(int idx) const struct DocExportStatus { Document::ExportStatus status; - std::set objs; + std::set objs; }; -static DocExportStatus _ExportStatus; +static DocExportStatus exportStatus; // Exception-safe exporting status setter class DocumentExporting { public: - explicit DocumentExporting(const std::vector& objs) + explicit DocumentExporting(const std::vector& objs) { - _ExportStatus.status = Document::Exporting; - _ExportStatus.objs.insert(objs.begin(), objs.end()); + exportStatus.status = Document::Exporting; + exportStatus.objs.insert(objs.begin(), objs.end()); } ~DocumentExporting() { - _ExportStatus.status = Document::NotExporting; - _ExportStatus.objs.clear(); + exportStatus.status = Document::NotExporting; + exportStatus.objs.clear(); } }; @@ -1199,16 +1163,16 @@ public: // at the same time. I see no benefits in distinguish which documents are // exporting, so just use a static variable for global status. But the // implementation can easily be changed here if necessary. -Document::ExportStatus Document::isExporting(const App::DocumentObject* obj) const +Document::ExportStatus Document::isExporting(const DocumentObject* obj) const { - if (_ExportStatus.status != Document::NotExporting - && (!obj || _ExportStatus.objs.find(obj) != _ExportStatus.objs.end())) { - return _ExportStatus.status; + if (exportStatus.status != Document::NotExporting + && ((obj == nullptr) || exportStatus.objs.find(obj) != exportStatus.objs.end())) { + return exportStatus.status; } return Document::NotExporting; } -void Document::exportObjects(const std::vector& obj, std::ostream& out) +void Document::exportObjects(const std::vector& obj, std::ostream& out) { DocumentExporting exporting(obj); @@ -1220,7 +1184,7 @@ void Document::exportObjects(const std::vector& obj, std:: FC_LOG("exporting " << o->getFullName()); if (!o->getPropertyByName("_ObjectUUID")) { auto prop = static_cast( - o->addDynamicProperty("App::PropertyUUID", + o->addDynamicProperty("PropertyUUID", "_ObjectUUID", nullptr, nullptr, @@ -1233,19 +1197,19 @@ void Document::exportObjects(const std::vector& obj, std:: Base::ZipWriter writer(out); writer.putNextEntry("Document.xml"); - writer.Stream() << "" << endl; + writer.Stream() << "" << '\n'; writer.Stream() << R"()" - << endl; + << Application::Config()["BuildVersionMajor"] << "." + << Application::Config()["BuildVersionMinor"] << "R" + << Application::Config()["BuildRevision"] << R"(" FileVersion="1">)" + << '\n'; // Add this block to have the same layout as for normal documents - writer.Stream() << "" << endl; - writer.Stream() << "" << endl; + writer.Stream() << "" << '\n'; + writer.Stream() << "" << '\n'; // writing the object types writeObjects(obj, writer); - writer.Stream() << "" << endl; + writer.Stream() << "" << '\n'; // Hook for others to add further data. signalExportObjects(obj, writer); @@ -1255,52 +1219,52 @@ void Document::exportObjects(const std::vector& obj, std:: d->hashers.clear(); } -#define FC_ATTR_DEPENDENCIES "Dependencies" -#define FC_ELEMENT_OBJECT_DEPS "ObjectDeps" -#define FC_ATTR_DEP_COUNT "Count" -#define FC_ATTR_DEP_OBJ_NAME "Name" -#define FC_ATTR_DEP_ALLOW_PARTIAL "AllowPartial" -#define FC_ELEMENT_OBJECT_DEP "Dep" +constexpr auto fcAttrDependencies {"Dependencies"}; +constexpr auto fcElementObjectDeps {"ObjectDeps"}; +constexpr auto fcAttrDepCount {"Count"}; +constexpr auto fcAttrDepObjName {"Name"}; +constexpr auto fcAttrDepAllowPartial {"AllowPartial"}; +constexpr auto fcElementObjectDep {"Dep"}; -void Document::writeObjects(const std::vector& obj, +void Document::writeObjects(const std::vector& obj, Base::Writer& writer) const { // writing the features types writer.incInd(); // indentation for 'Objects count' writer.Stream() << writer.ind() << "" << endl; + writer.Stream() << "\">" << '\n'; writer.incInd(); // indentation for 'Object type' - if (!isExporting(nullptr)) { - for (auto o : obj) { + if (isExporting(nullptr) == 0U) { + for (const auto o : obj) { const auto& outList = o->getOutList(DocumentObject::OutListNoHidden | DocumentObject::OutListNoXLinked); writer.Stream() << writer.ind() - << "<" FC_ELEMENT_OBJECT_DEPS " " FC_ATTR_DEP_OBJ_NAME "=\"" - << o->getNameInDocument() << "\" " FC_ATTR_DEP_COUNT "=\"" + << "<" << fcElementObjectDeps << " " << fcAttrDepObjName << "=\"" + << o->getNameInDocument() << "\" " << fcAttrDepCount << "=\"" << outList.size(); if (outList.empty()) { - writer.Stream() << "\"/>" << endl; + writer.Stream() << "\"/>" << '\n'; continue; } - int partial = o->canLoadPartial(); + const int partial = o->canLoadPartial(); if (partial > 0) { - writer.Stream() << "\" " FC_ATTR_DEP_ALLOW_PARTIAL << "=\"" << partial; + writer.Stream() << "\" " << fcAttrDepAllowPartial << "=\"" << partial; } - writer.Stream() << "\">" << endl; + writer.Stream() << "\">" << '\n'; writer.incInd(); - for (auto dep : outList) { - auto name = dep ? dep->getNameInDocument() : ""; + for (const auto dep : outList) { + const auto name = dep ? dep->getNameInDocument() : ""; writer.Stream() << writer.ind() - << "<" FC_ELEMENT_OBJECT_DEP " " FC_ATTR_DEP_OBJ_NAME "=\"" - << (name ? name : "") << "\"/>" << endl; + << "<" << fcElementObjectDep << " " << fcAttrDepObjName << "=\"" + << (name ? name : "") << "\"/>" << '\n'; } writer.decInd(); - writer.Stream() << writer.ind() << "" << endl; + writer.Stream() << writer.ind() << "" << '\n'; } } @@ -1323,19 +1287,19 @@ void Document::writeObjects(const std::vector& obj, } if ((*it)->testStatus(ObjectStatus::Error)) { writer.Stream() << "Invalid=\"1\" "; - auto desc = getErrorDescription(*it); + const auto desc = getErrorDescription(*it); if (desc) { writer.Stream() << "Error=\"" << Property::encodeAttribute(desc) << "\" "; } } - writer.Stream() << "/>" << endl; + writer.Stream() << "/>" << '\n'; } writer.decInd(); // indentation for 'Object type' - writer.Stream() << writer.ind() << "" << endl; + writer.Stream() << writer.ind() << "" << '\n'; // writing the features itself - writer.Stream() << writer.ind() << "" << endl; + writer.Stream() << writer.ind() << "" << '\n'; writer.incInd(); // indentation for 'Object name' for (it = obj.begin(); it != obj.end(); ++it) { @@ -1344,13 +1308,13 @@ void Document::writeObjects(const std::vector& obj, writer.Stream() << " Extensions=\"True\""; } - writer.Stream() << ">" << endl; + writer.Stream() << ">" << '\n'; (*it)->Save(writer); - writer.Stream() << writer.ind() << "" << endl; + writer.Stream() << writer.ind() << "" << '\n'; } writer.decInd(); // indentation for 'Object name' - writer.Stream() << writer.ind() << "" << endl; + writer.Stream() << writer.ind() << "" << '\n'; writer.decInd(); // indentation for 'Objects count' } @@ -1360,16 +1324,16 @@ struct DepInfo int canLoadPartial = 0; }; -static void _loadDeps(const std::string& name, +static void loadDeps(const std::string& name, std::unordered_map& objs, const std::unordered_map& deps) { - auto it = deps.find(name); + const auto it = deps.find(name); if (it == deps.end()) { objs.emplace(name, true); return; } - if (it->second.canLoadPartial) { + if (it->second.canLoadPartial != 0) { if (it->second.canLoadPartial == 1) { // canLoadPartial==1 means all its children will be created but not // restored, i.e. exists as if newly created object, and therefore no @@ -1387,59 +1351,59 @@ static void _loadDeps(const std::string& name, objs[name] = true; // If cannot load partial, then recurse to load all children dependency for (auto& dep : it->second.deps) { - auto it = objs.find(dep); - if (it != objs.end() && it->second) { + if (auto found = objs.find(dep); found != objs.end() && found->second) { continue; } - _loadDeps(dep, objs, deps); + loadDeps(dep, objs, deps); } } -std::vector Document::readObjects(Base::XMLReader& reader) +std::vector Document::readObjects(Base::XMLReader& reader) { d->touchedObjs.clear(); bool keepDigits = testStatus(Document::KeepTrailingDigits); setStatus(Document::KeepTrailingDigits, !reader.doNameMapping()); - std::vector objs; + std::vector objs; // read the object types reader.readElement("Objects"); - int Cnt = reader.getAttribute("Count"); + int Cnt = static_cast(reader.getAttribute("Count")); - if (!reader.hasAttribute(FC_ATTR_DEPENDENCIES)) { + if (!reader.hasAttribute(fcAttrDependencies)) { d->partialLoadObjects.clear(); } else if (!d->partialLoadObjects.empty()) { std::unordered_map deps; for (int i = 0; i < Cnt; i++) { - reader.readElement(FC_ELEMENT_OBJECT_DEPS); - int dcount = reader.getAttribute(FC_ATTR_DEP_COUNT); - if (!dcount) { + reader.readElement(fcElementObjectDeps); + int dcount = static_cast(reader.getAttribute(fcAttrDepCount)); + if (dcount == 0) { continue; } - auto& info = deps[reader.getAttribute(FC_ATTR_DEP_OBJ_NAME)]; - if (reader.hasAttribute(FC_ATTR_DEP_ALLOW_PARTIAL)) { - info.canLoadPartial = reader.getAttribute(FC_ATTR_DEP_ALLOW_PARTIAL); + auto& info = deps[reader.getAttribute(fcAttrDepObjName)]; + if (reader.hasAttribute(fcAttrDepAllowPartial)) { + info.canLoadPartial = + static_cast(reader.getAttribute(fcAttrDepAllowPartial)); } for (int j = 0; j < dcount; ++j) { - reader.readElement(FC_ELEMENT_OBJECT_DEP); - const char* name = reader.getAttribute(FC_ATTR_DEP_OBJ_NAME); + reader.readElement(fcElementObjectDep); + const char* name = reader.getAttribute(fcAttrDepObjName); if (!Base::Tools::isNullOrEmpty(name)) { info.deps.insert(name); } } - reader.readEndElement(FC_ELEMENT_OBJECT_DEPS); + reader.readEndElement(fcElementObjectDeps); } - std::vector objs; - objs.reserve(d->partialLoadObjects.size()); + std::vector strings; + strings.reserve(d->partialLoadObjects.size()); for (auto& v : d->partialLoadObjects) { - objs.emplace_back(v.first.c_str()); + strings.emplace_back(v.first.c_str()); } - for (auto& name : objs) { - _loadDeps(name, d->partialLoadObjects, deps); + for (auto& name : strings) { + loadDeps(name, d->partialLoadObjects, deps); } - if (Cnt > (int)d->partialLoadObjects.size()) { + if (Cnt > static_cast(d->partialLoadObjects.size())) { setStatus(Document::PartialDoc, true); } else { @@ -1487,7 +1451,7 @@ std::vector Document::readObjects(Base::XMLReader& reader) // correctly unmap the names. auto pos = name.find('@'); std::string _obj_name; - const char* obj_name; + const char* obj_name {nullptr}; if (pos != std::string::npos) { _obj_name = name.substr(0, pos); obj_name = _obj_name.c_str(); @@ -1501,7 +1465,7 @@ std::vector Document::readObjects(Base::XMLReader& reader) // otherwise we may cause a dependency to itself // Example: Object 'Cut001' references object 'Cut' and removing the // digits we make an object 'Cut' referencing itself. - App::DocumentObject* obj = + DocumentObject* obj = addObject(type.c_str(), obj_name, /*isNew=*/false, viewType.c_str(), partial); if (obj) { if (lastId < obj->_Id) { @@ -1541,14 +1505,13 @@ std::vector Document::readObjects(Base::XMLReader& reader) // read the features itself reader.clearPartialRestoreDocumentObject(); reader.readElement("ObjectData"); - Cnt = reader.getAttribute("Count"); + Cnt = static_cast(reader.getAttribute("Count")); for (int i = 0; i < Cnt; i++) { reader.readElement("Object"); std::string name = reader.getName(reader.getAttribute("name")); - DocumentObject* pObj = getObject(name.c_str()); - if (pObj + if (DocumentObject* pObj = getObject(name.c_str()); pObj && !pObj->testStatus( - App::PartialObject)) { // check if this feature has been registered + PartialObject)) { // check if this feature has been registered pObj->setStatus(ObjectStatus::Restore, true); try { FC_TRACE("restoring " << pObj->getFullName()); @@ -1588,7 +1551,7 @@ std::vector Document::readObjects(Base::XMLReader& reader) return objs; } -void Document::addRecomputeObject(DocumentObject* obj) +void Document::addRecomputeObject(DocumentObject* obj) // NOLINT { if (testStatus(Status::Restoring) && obj) { setStatus(Status::RecomputeOnRestore, true); @@ -1597,7 +1560,7 @@ void Document::addRecomputeObject(DocumentObject* obj) } } -std::vector Document::importObjects(Base::XMLReader& reader) +std::vector Document::importObjects(Base::XMLReader& reader) { d->hashers.clear(); Base::FlagToggler<> flag(globalIsRestoring, false); @@ -1605,8 +1568,8 @@ std::vector Document::importObjects(Base::XMLReader& reade Base::ObjectStatusLocker restoreBit2(Status::Importing, this); ExpressionParser::ExpressionImporter expImporter(reader); reader.readElement("Document"); - long scheme = reader.getAttribute("SchemaVersion"); - reader.DocumentSchema = scheme; + const long scheme = reader.getAttribute("SchemaVersion"); + reader.DocumentSchema = static_cast(scheme); if (reader.hasAttribute("ProgramVersion")) { reader.ProgramVersion = reader.getAttribute("ProgramVersion"); } @@ -1614,18 +1577,18 @@ std::vector Document::importObjects(Base::XMLReader& reade reader.ProgramVersion = "pre-0.14"; } if (reader.hasAttribute("FileVersion")) { - reader.FileVersion = reader.getAttribute("FileVersion"); + reader.FileVersion = static_cast(reader.getAttribute("FileVersion")); } else { reader.FileVersion = 0; } - std::vector objs = readObjects(reader); - for (auto o : objs) { + std::vector objs = readObjects(reader); + for (const auto o : objs) { if (o && o->isAttachedToDocument()) { - o->setStatus(App::ObjImporting, true); + o->setStatus(ObjImporting, true); FC_LOG("importing " << o->getFullName()); - if (auto propUUID = + if (const auto propUUID = freecad_cast(o->getPropertyByName("_ObjectUUID"))) { auto propSource = freecad_cast(o->getPropertyByName("_SourceUUID")); @@ -1652,9 +1615,9 @@ std::vector Document::importObjects(Base::XMLReader& reade signalFinishImportObjects(objs); - for (auto o : objs) { + for (const auto o : objs) { if (o && o->isAttachedToDocument()) { - o->setStatus(App::ObjImporting, false); + o->setStatus(ObjImporting, false); } } @@ -1667,9 +1630,8 @@ unsigned int Document::getMemSize() const unsigned int size = 0; // size of the DocObjects in the document - std::vector::const_iterator it; - for (it = d->objectArray.begin(); it != d->objectArray.end(); ++it) { - size += (*it)->getMemSize(); + for (const auto & it : d->objectArray) { + size += it->getMemSize(); } size += d->Hasher->getMemSize(); @@ -1689,11 +1651,11 @@ static std::string checkFileName(const char* file) // Append extension if missing. This option is added for security reason, so // that the user won't accidentally overwrite other file that may be critical. - if (App::GetApplication() + if (GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetBool("CheckExtension", true)) { const char* ext = strrchr(file, '.'); - if (!ext || !boost::iequals(ext + 1, "fcstd")) { + if ((ext == nullptr) || !boost::iequals(ext + 1, "fcstd")) { if (ext && ext[1] == 0) { fn += "FCStd"; } @@ -1707,8 +1669,8 @@ static std::string checkFileName(const char* file) bool Document::saveAs(const char* _file) { - std::string file = checkFileName(_file); - Base::FileInfo fi(file.c_str()); + const std::string file = checkFileName(_file); + const Base::FileInfo fi(file.c_str()); if (this->FileName.getStrValue() != file) { this->FileName.setValue(file); this->Label.setValue(fi.fileNamePure()); @@ -1718,14 +1680,10 @@ bool Document::saveAs(const char* _file) return save(); } -bool Document::saveCopy(const char* _file) const +bool Document::saveCopy(const char* file) const { - std::string file = checkFileName(_file); - if (this->FileName.getStrValue() != file) { - bool result = saveToFile(file.c_str()); - return result; - } - return false; + const std::string checked = checkFileName(file); + return this->FileName.getStrValue() != checked ? saveToFile(checked.c_str()) : false; } // Save the document under the name it has been opened @@ -1745,16 +1703,16 @@ bool Document::save() TipName.setValue(Tip.getValue()->getNameInDocument()); } - std::string LastModifiedDateString = Base::Tools::currentDateTimeString(); + const std::string LastModifiedDateString = Base::Tools::currentDateTimeString(); LastModifiedDate.setValue(LastModifiedDateString.c_str()); // set author if needed - bool saveAuthor = - App::GetApplication() + const bool saveAuthor = + GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetBool("prefSetAuthorOnSave", false); if (saveAuthor) { - std::string Author = - App::GetApplication() + const std::string Author = + GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetASCII("prefAuthor", ""); LastModifiedBy.setValue(Author.c_str()); @@ -1778,22 +1736,17 @@ public: TimeStamp }; BackupPolicy() - { - policy = Standard; - numberOfFiles = 1; - useFCBakExtension = true; - saveBackupDateFormat = "%Y%m%d-%H%M%S"; - } + {} ~BackupPolicy() = default; - void setPolicy(Policy p) + void setPolicy(const Policy p) { policy = p; } - void setNumberOfFiles(int count) + void setNumberOfFiles(const int count) { numberOfFiles = count; } - void useBackupExtension(bool on) + void useBackupExtension(const bool on) { useFCBakExtension = on; } @@ -1814,11 +1767,10 @@ public: } private: - void applyStandard(const std::string& sourcename, const std::string& targetname) + void applyStandard(const std::string& sourcename, const std::string& targetname) const { // if saving the project data succeeded rename to the actual file name - Base::FileInfo fi(targetname); - if (fi.exists()) { + if (Base::FileInfo fi(targetname); fi.exists()) { if (numberOfFiles > 0) { int nSuff = 0; std::string fn = fi.fileName(); @@ -1826,8 +1778,7 @@ private: std::vector backup; std::vector files = di.getDirectoryContent(); for (const Base::FileInfo& it : files) { - std::string file = it.fileName(); - if (file.substr(0, fn.length()) == fn) { + if (std::string file = it.fileName(); file.substr(0, fn.length()) == fn) { // starts with the same file name std::string suf(file.substr(fn.length())); if (!suf.empty()) { @@ -1835,13 +1786,14 @@ private: if (nPos == std::string::npos) { // store all backup files backup.push_back(it); - nSuff = std::max(nSuff, std::atol(suf.c_str())); + nSuff = + std::max(nSuff, static_cast(std::atol(suf.c_str()))); } } } } - if (!backup.empty() && (int)backup.size() >= numberOfFiles) { + if (!backup.empty() && static_cast(backup.size()) >= numberOfFiles) { // delete the oldest backup file we found Base::FileInfo del = backup.front(); for (const Base::FileInfo& it : backup) { @@ -1869,8 +1821,7 @@ private: } } - Base::FileInfo tmp(sourcename); - if (!tmp.renameFile(targetname.c_str())) { + if (Base::FileInfo tmp(sourcename); !tmp.renameFile(targetname.c_str())) { throw Base::FileException("Cannot rename tmp save file to project file", Base::FileInfo(targetname)); } @@ -1899,7 +1850,7 @@ private: boost::replace_all(saveBackupDateFormat, ".", "-"); { // Remove all extra backups - std::string fn = fi.fileName(); + std::string filename = fi.fileName(); Base::FileInfo di(fi.dirPath()); std::vector backup; std::vector files = di.getDirectoryContent(); @@ -1911,14 +1862,14 @@ private: std::transform(fextUp.begin(), fextUp.end(), fextUp.begin(), - (int (*)(int))toupper); + static_cast(toupper)); // re-enforcing identification of the backup file // old case : the name starts with the full name of the project and // follows with numbers - if ((startsWith(file, fn) && (file.length() > fn.length()) - && checkDigits(file.substr(fn.length()))) + if ((startsWith(file, filename) && (file.length() > filename.length()) + && checkDigits(file.substr(filename.length()))) || // .FCBak case : The bame starts with the base name of the project + // "." @@ -1930,7 +1881,7 @@ private: } } - if (!backup.empty() && (int)backup.size() >= numberOfFiles) { + if (!backup.empty() && static_cast(backup.size()) >= numberOfFiles) { std::sort(backup.begin(), backup.end(), fileComparisonByDate); // delete the oldest backup file we found // Base::FileInfo del = backup.front(); @@ -1957,7 +1908,7 @@ private: // create a new backup file { - int ext = 1; + int ext2 = 1; if (useFCBakExtension) { std::stringstream str; Base::TimeInfo ti = fi.lastModified(); @@ -1987,28 +1938,28 @@ private: } if (!done) { - while (ext < numberOfFiles + 10) { - if (renameFileNoErase(fi, fn + std::to_string(ext) + ".FCBak")) { + while (ext2 < numberOfFiles + 10) { + if (renameFileNoErase(fi, fn + std::to_string(ext2) + ".FCBak")) { break; } - ext++; + ext2++; } } } else { // changed but simpler and solves also the delay sometimes introduced by // google drive - while (ext < numberOfFiles + 10) { + while (ext2 < numberOfFiles + 10) { // linux just replace the file if exists, and then the existence is to // be tested before rename - if (renameFileNoErase(fi, fi.filePath() + std::to_string(ext))) { + if (renameFileNoErase(fi, fi.filePath() + std::to_string(ext2))) { break; } - ext++; + ext2++; } } - if (ext >= numberOfFiles + 10) { + if (ext2 >= numberOfFiles + 10) { Base::Console().error( "File not saved: Cannot rename project file to backup file\n"); // throw Base::FileException("File not saved: Cannot rename project file to @@ -2052,27 +2003,27 @@ private: bool checkValidString(const std::string& cmpl, const boost::regex& e) const { boost::smatch what; - bool res = boost::regex_search(cmpl, what, e); + const bool res = boost::regex_search(cmpl, what, e); return res; } bool checkValidComplement(const std::string& file, const std::string& pbn, const std::string& ext) const { - std::string cmpl = + const std::string cmpl = file.substr(pbn.length(), file.length() - pbn.length() - ext.length() - 1); - boost::regex e(R"(^[^.]*$)"); + const boost::regex e(R"(^[^.]*$)"); return checkValidString(cmpl, e); } bool checkDigits(const std::string& cmpl) const { - boost::regex e(R"(^[0-9]*$)"); + const boost::regex e(R"(^[0-9]*$)"); return checkValidString(cmpl, e); } bool renameFileNoErase(Base::FileInfo fi, const std::string& newName) { // linux just replaces the file if it exists, so the existence is to be tested before rename - Base::FileInfo nf(newName); + const Base::FileInfo nf(newName); if (!nf.exists()) { return fi.renameFile(newName.c_str()); } @@ -2080,10 +2031,10 @@ private: } private: - Policy policy; - int numberOfFiles; - bool useFCBakExtension; - std::string saveBackupDateFormat; + Policy policy {Standard}; + int numberOfFiles {1}; + bool useFCBakExtension {true}; + std::string saveBackupDateFormat {"%Y%m%d-%H%M%S"}; }; } // namespace App @@ -2091,12 +2042,12 @@ bool Document::saveToFile(const char* filename) const { signalStartSave(*this, filename); - auto hGrp = App::GetApplication().GetParameterGroupByPath( + auto hGrp = GetApplication().GetParameterGroupByPath( "User parameter:BaseApp/Preferences/Document"); - int compression = hGrp->GetInt("CompressionLevel", 7); + int compression = static_cast(hGrp->GetInt("CompressionLevel", 7)); compression = Base::clamp(compression, Z_NO_COMPRESSION, Z_BEST_COMPRESSION); - bool policy = App::GetApplication() + bool policy = GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetBool("BackupPolicy", true); @@ -2141,11 +2092,11 @@ bool Document::saveToFile(const char* filename) const fn += "."; fn += uuid; } - Base::FileInfo tmp(fn); // open extra scope to close ZipWriter properly { + Base::FileInfo tmp(fn); Base::ofstream file(tmp, std::ios::out | std::ios::binary); Base::ZipWriter writer(file); @@ -2161,11 +2112,11 @@ bool Document::saveToFile(const char* filename) const writer.setMode("BinaryBrep"); } - writer.Stream() << "" << endl - << "" << endl; + << '\n' + << "-->" << '\n'; Document::Save(writer); // Special handling for Gui document. @@ -2186,35 +2137,35 @@ bool Document::saveToFile(const char* filename) const if (policy) { // if saving the project data succeeded rename to the actual file name - int count_bak = App::GetApplication() + int count_bak = static_cast(GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") - ->GetInt("CountBackupFiles", 1); - bool backup = App::GetApplication() + ->GetInt("CountBackupFiles", 1)); + bool backup = GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetBool("CreateBackupFiles", true); if (!backup) { count_bak = -1; } bool useFCBakExtension = - App::GetApplication() + GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetBool("UseFCBakExtension", true); std::string saveBackupDateFormat = - App::GetApplication() + GetApplication() .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document") ->GetASCII("SaveBackupDateFormat", "%Y%m%d-%H%M%S"); - BackupPolicy policy; + BackupPolicy backupPolicy; if (useFCBakExtension) { - policy.setPolicy(BackupPolicy::TimeStamp); - policy.useBackupExtension(useFCBakExtension); - policy.setDateFormat(saveBackupDateFormat); + backupPolicy.setPolicy(BackupPolicy::TimeStamp); + backupPolicy.useBackupExtension(useFCBakExtension); + backupPolicy.setDateFormat(saveBackupDateFormat); } else { - policy.setPolicy(BackupPolicy::Standard); + backupPolicy.setPolicy(BackupPolicy::Standard); } - policy.setNumberOfFiles(count_bak); - policy.apply(fn, nativePath); + backupPolicy.setNumberOfFiles(count_bak); + backupPolicy.apply(fn, nativePath); } signalFinishSave(*this, filename); @@ -2348,7 +2299,7 @@ void Document::restore(const char* filename, } } -bool Document::afterRestore(bool checkPartial) +bool Document::afterRestore(const bool checkPartial) { Base::FlagToggler<> flag(globalIsRestoring, false); if (!afterRestore(d->objectArray, checkPartial)) { @@ -2374,7 +2325,7 @@ bool Document::afterRestore(const std::vector& objArray, bool c // Property::afterRestore() interface to let them sort it out. Note, this // API is not called in object dedpenency order, because the order // information is not ready yet. - std::map> propMap; + std::map> propMap; for (auto obj : objArray) { auto& props = propMap[obj]; obj->getPropertyList(props); @@ -2436,9 +2387,9 @@ bool Document::afterRestore(const std::vector& objArray, bool c obj->getPropertyList(props); for (auto prop : props) { auto link = freecad_cast(prop); - int res; + int res {0}; std::string errMsg; - if (link && (res = link->checkRestore(&errMsg))) { + if (link && ((res = link->checkRestore(&errMsg)) != 0)) { d->touchedObjs.insert(obj); if (res == 1 || checkPartial) { FC_WARN(obj->getFullName() << '.' << prop->getName() << ": " << errMsg); @@ -2460,7 +2411,7 @@ bool Document::afterRestore(const std::vector& objArray, bool c // partial document touched, signal full reload return false; } - else if (!d->touchedObjs.contains(obj)) { + if (!d->touchedObjs.contains(obj)) { obj->purgeTouched(); } @@ -2473,7 +2424,7 @@ bool Document::afterRestore(const std::vector& objArray, bool c bool Document::isSaved() const { - std::string name = FileName.getValue(); + const std::string name = FileName.getValue(); return !name.empty(); } @@ -2520,16 +2471,16 @@ const char* Document::getFileName() const } /// Remove all modifications. After this call The document becomes valid again. -void Document::purgeTouched() +void Document::purgeTouched() // NOLINT { - for (auto It : d->objectArray) { + for (const auto It : d->objectArray) { It->purgeTouched(); } } bool Document::isTouched() const { - for (auto It : d->objectArray) { + for (const auto It : d->objectArray) { if (It->isTouched()) { return true; } @@ -2550,7 +2501,7 @@ vector Document::getTouched() const return result; } -void Document::setClosable(bool c) +void Document::setClosable(const bool c) // NOLINT { setStatus(Document::Closable, c); } @@ -2567,69 +2518,65 @@ int Document::countObjects() const void Document::getLinksTo(std::set& links, const DocumentObject* obj, - int options, - int maxCount, + const int options, + const int maxCount, const std::vector& objs) const { - std::map> linkMap; + std::map> linkMap; for (auto o : !objs.empty() ? objs : d->objectArray) { if (o == obj) { continue; } auto linked = o; - if (options & GetLinkArrayElement) { + if ((options & GetLinkArrayElement) != 0) { linked = o->getLinkedObject(false); } else { - auto ext = o->getExtensionByType(true); - if (ext) { - linked = ext->getTrueLinkedObject(false, nullptr, 0, true); - } - else { - linked = o->getLinkedObject(false); - } + const auto ext = o->getExtensionByType(true); + linked = + ext ? ext->getTrueLinkedObject(false, nullptr, 0, true) : o->getLinkedObject(false); } if (linked && linked != o) { - if (options & GetLinkRecursive) { + if ((options & GetLinkRecursive) != 0) { linkMap[linked].push_back(o); } else if (linked == obj || !obj) { - if ((options & GetLinkExternal) && linked->getDocument() == o->getDocument()) { + if (((options & GetLinkExternal) != 0) && linked->getDocument() == o->getDocument()) { continue; } - else if (options & GetLinkedObject) { + if ((options & GetLinkedObject) != 0) { links.insert(linked); } else { links.insert(o); } - if (maxCount && maxCount <= (int)links.size()) { + if ((maxCount != 0) && maxCount <= static_cast(links.size())) { return; } } } } - if (!(options & GetLinkRecursive)) { + if ((options & GetLinkRecursive) == 0) { return; } std::vector current(1, obj); for (int depth = 0; !current.empty(); ++depth) { - if (!GetApplication().checkLinkDepth(depth, MessageOption::Error)) { + if (GetApplication().checkLinkDepth(depth, MessageOption::Error) == 0) { break; } std::vector next; - for (const App::DocumentObject* o : current) { + for (const DocumentObject* o : current) { auto iter = linkMap.find(o); if (iter == linkMap.end()) { continue; } - for (App::DocumentObject* link : iter->second) { + for (DocumentObject* link : iter->second) { if (links.insert(link).second) { - if (maxCount && maxCount <= (int)links.size()) { + if ((maxCount != 0) && maxCount <= static_cast(links.size())) { return; } next.push_back(link); @@ -2638,7 +2585,6 @@ void Document::getLinksTo(std::set& links, } current = std::move(next); } - return; } bool Document::hasLinksTo(const DocumentObject* obj) const @@ -2648,18 +2594,18 @@ bool Document::hasLinksTo(const DocumentObject* obj) const return !links.empty(); } -std::vector Document::getInList(const DocumentObject* me) const +std::vector Document::getInList(const DocumentObject* me) const { // result list - std::vector result; + std::vector result; // go through all objects - for (const auto& It : d->objectMap) { + for (const auto& [name, object] : d->objectMap) { // get the outList and search if me is in that list - std::vector OutList = It.second->getOutList(); - for (auto obj : OutList) { + std::vector OutList = object->getOutList(); + for (const auto obj : OutList) { if (obj && obj == me) { // add the parent object - result.push_back(It.second); + result.push_back(object); } } } @@ -2679,9 +2625,9 @@ std::vector Document::getInList(const DocumentObject* me) // assumption is broken by the introduction of PropertyXLink which can link to // external object. // -static void _buildDependencyList(const std::vector& objectArray, - int options, - std::vector* depObjs, +static void buildDependencyList(const std::vector& objectArray, + const int options, + std::vector* depObjs, DependencyList* depList, std::map* objectMap, bool* touchCheck = nullptr) @@ -2696,58 +2642,58 @@ static void _buildDependencyList(const std::vector& object depList->clear(); } - int op = (options & Document::DepNoXLinked) ? DocumentObject::OutListNoXLinked : 0; + const int op = ((options & Document::DepNoXLinked) != 0) ? DocumentObject::OutListNoXLinked : 0; for (auto obj : objectArray) { objs.push_back(obj); while (!objs.empty()) { - auto obj = objs.front(); + auto objF = objs.front(); objs.pop_front(); - if (!obj || !obj->isAttachedToDocument()) { + if (!objF || !objF->isAttachedToDocument()) { continue; } - auto it = outLists.find(obj); + auto it = outLists.find(objF); if (it != outLists.end()) { continue; } if (touchCheck) { - if (obj->isTouched() || obj->mustExecute()) { + if (objF->isTouched() || (objF->mustExecute() != 0)) { // early termination on touch check *touchCheck = true; return; } } if (depObjs) { - depObjs->push_back(obj); + depObjs->push_back(objF); } if (objectMap && depList) { - (*objectMap)[obj] = add_vertex(*depList); + (*objectMap)[objF] = add_vertex(*depList); } - auto& outList = outLists[obj]; - outList = obj->getOutList(op); + auto& outList = outLists[objF]; + outList = objF->getOutList(op); objs.insert(objs.end(), outList.begin(), outList.end()); } } if (objectMap && depList) { - for (const auto& v : outLists) { - for (auto obj : v.second) { + for (const auto& [key, objects] : outLists) { + for (auto obj : objects) { if (obj && obj->isAttachedToDocument()) { - add_edge((*objectMap)[v.first], (*objectMap)[obj], *depList); + add_edge((*objectMap)[key], (*objectMap)[obj], *depList); } } } } } -std::vector -Document::getDependencyList(const std::vector& objectArray, int options) +std::vector +Document::getDependencyList(const std::vector& objs, int options) { - std::vector ret; - if (!(options & DepSort)) { - _buildDependencyList(objectArray, options, &ret, nullptr, nullptr); + std::vector ret; + if ((options & DepSort) == 0) { + buildDependencyList(objs, options, &ret, nullptr, nullptr); return ret; } @@ -2755,7 +2701,7 @@ Document::getDependencyList(const std::vector& objectArray std::map objectMap; std::map vertexMap; - _buildDependencyList(objectArray, options, nullptr, &depList, &objectMap); + buildDependencyList(objs, options, nullptr, &depList, &objectMap); for (auto& v : objectMap) { vertexMap[v.second] = v.first; @@ -2766,7 +2712,7 @@ Document::getDependencyList(const std::vector& objectArray boost::topological_sort(depList, std::front_inserter(make_order)); } catch (const std::exception& e) { - if (options & DepNoCycle) { + if ((options & DepNoCycle) != 0) { // Use boost::strong_components to find cycles. It groups strongly // connected vertices as components, and therefore each component // forms a cycle. @@ -2783,90 +2729,90 @@ Document::getDependencyList(const std::vector& objectArray FC_ERR("Dependency cycles: "); std::ostringstream ss; - ss << std::endl; - for (auto& v : components) { - if (v.second.size() == 1) { + ss << '\n'; + for (auto& [key, vertexes] : components) { + if (vertexes.size() == 1) { // For components with only one member, we still need to // check if there it is self looping. - auto it = vertexMap.find(v.second[0]); + auto it = vertexMap.find(vertexes[0]); if (it == vertexMap.end()) { continue; } // Try search the object in its own out list for (auto obj : it->second->getOutList()) { if (obj == it->second) { - ss << std::endl << it->second->getFullName() << std::endl; + ss << '\n' << it->second->getFullName() << '\n'; break; } } continue; } // For components with more than one member, they form a loop together - for (size_t i = 0; i < v.second.size(); ++i) { - auto it = vertexMap.find(v.second[i]); + for (size_t i = 0; i < vertexes.size(); ++i) { + auto it = vertexMap.find(vertexes[i]); if (it == vertexMap.end()) { continue; } if (i % 6 == 0) { - ss << std::endl; + ss << '\n'; } ss << it->second->getFullName() << ", "; } - ss << std::endl; + ss << '\n'; } FC_ERR(ss.str()); FC_THROWM(Base::RuntimeError, e.what()); } FC_ERR(e.what()); - ret = DocumentP::partialTopologicalSort(objectArray); + ret = DocumentP::partialTopologicalSort(objs); std::reverse(ret.begin(), ret.end()); return ret; } - for (std::list::reverse_iterator i = make_order.rbegin(); i != make_order.rend(); ++i) { + for (auto i = make_order.rbegin(); i != make_order.rend(); ++i) { ret.push_back(vertexMap[*i]); } return ret; } -std::vector Document::getDependentDocuments(bool sort) +std::vector Document::getDependentDocuments(const bool sort) { return getDependentDocuments({this}, sort); } -std::vector Document::getDependentDocuments(std::vector pending, - bool sort) +std::vector Document::getDependentDocuments(std::vector docs, + const bool sort) { DependencyList depList; std::map docMap; std::map vertexMap; - std::vector ret; - if (pending.empty()) { + std::vector ret; + if (docs.empty()) { return ret; } auto outLists = PropertyXLink::getDocumentOutList(); - std::set docs; - docs.insert(pending.begin(), pending.end()); + std::set docSet; + docSet.insert(docs.begin(), docs.end()); if (sort) { - for (auto doc : pending) { + for (auto doc : docs) { docMap[doc] = add_vertex(depList); } } - while (!pending.empty()) { - auto doc = pending.back(); - pending.pop_back(); + while (!docs.empty()) { + auto doc = docs.back(); + docs.pop_back(); auto it = outLists.find(doc); if (it == outLists.end()) { continue; } - auto& vertex = docMap[doc]; + const auto& vertex = docMap[doc]; for (auto depDoc : it->second) { - if (docs.insert(depDoc).second) { - pending.push_back(depDoc); + if (docSet.insert(depDoc).second) { + docs.push_back(depDoc); if (sort) { docMap[depDoc] = add_vertex(depList); } @@ -2876,7 +2822,7 @@ std::vector Document::getDependentDocuments(std::vector Document::getDependentDocuments(std::vector& objs) +void Document::_rebuildDependencyList(const std::vector& objs) { (void)objs; } @@ -2914,25 +2860,25 @@ void Document::_rebuildDependencyList(const std::vector& o */ void Document::renameObjectIdentifiers( - const std::map& paths, - const std::function& selector) + const std::map& paths, + const std::function& selector) // NOLINT { - std::map extendedPaths; + std::map extendedPaths; - std::map::const_iterator it = paths.begin(); + auto it = paths.begin(); while (it != paths.end()) { extendedPaths[it->first.canonicalPath()] = it->second.canonicalPath(); ++it; } - for (auto it : d->objectArray) { - if (selector(it)) { - it->renameObjectIdentifiers(extendedPaths); + for (const auto object : d->objectArray) { + if (selector(object)) { + object->renameObjectIdentifiers(extendedPaths); } } } -int Document::recompute(const std::vector& objs, +int Document::recompute(const std::vector& objs, bool force, bool* hasError, int options) @@ -2974,7 +2920,6 @@ int Document::recompute(const std::vector& objs, Base::ObjectStatusLocker exe(Document::Recomputing, this); signalBeforeRecompute(*this); -#if 0 ////////////////////////////////////////////////////////////////////////// // FIXME Comment by Realthunder: // the topologicalSrot() below cannot handle partial recompute, haven't got @@ -2984,18 +2929,20 @@ int Document::recompute(const std::vector& objs, // it report for cyclic dependency. ////////////////////////////////////////////////////////////////////////// - // get the sorted vector of all dependent objects and go though it from the end - auto depObjs = getDependencyList(objs.empty()?d->objectArray:objs); - vector topoSortedObjects = topologicalSort(depObjs); - if (topoSortedObjects.size() != depObjs.size()){ - cerr << "App::Document::recompute(): cyclic dependency detected" << endl; - topoSortedObjects = d->partialTopologicalSort(depObjs); - } - std::reverse(topoSortedObjects.begin(),topoSortedObjects.end()); -#else + /* // get the sorted vector of all dependent objects and go though it from the end + auto depObjs = getDependencyList(objs.empty()?d->objectArray:objs); + vector topoSortedObjects = topologicalSort(depObjs); + if (topoSortedObjects.size() != depObjs.size()){ + cerr << "Document::recompute(): cyclic dependency detected" << '\n'; + topoSortedObjects = d->partialTopologicalSort(depObjs); + } + std::reverse(topoSortedObjects.begin(),topoSortedObjects.end()); + */ + + // alt: auto topoSortedObjects = getDependencyList(objs.empty() ? d->objectArray : objs, DepSort | options); -#endif + for (auto obj : topoSortedObjects) { obj->setStatus(ObjectStatus::PendingRecompute, true); } @@ -3004,12 +2951,11 @@ int Document::recompute(const std::vector& objs, GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document"); bool canAbort = hGrp->GetBool("CanAbortRecompute", true); - std::set filter; - size_t idx = 0; - FC_TIME_INIT(t2); try { + std::set filter; + size_t idx = 0; // maximum two passes to allow some form of dependency inversion for (int passes = 0; passes < 2 && idx < topoSortedObjects.size(); ++passes) { std::unique_ptr seq; @@ -3029,7 +2975,7 @@ int Document::recompute(const std::vector& objs, doRecompute = true; ++objectCount; int res = _recomputeFeature(obj); - if (res) { + if (res != 0) { if (hasError) { *hasError = true; } @@ -3108,9 +3054,9 @@ int Document::recompute(const std::vector& objs, } for (auto doc : GetApplication().getDocuments()) { - decltype(doc->d->pendingRemove) objs; - objs.swap(doc->d->pendingRemove); - for (auto& o : objs) { + decltype(doc->d->pendingRemove) objects; + objects.swap(doc->d->pendingRemove); + for (auto& o : objects) { try { if (auto obj = o.getObject()) { obj->getDocument()->removeObject(obj->getNameInDocument()); @@ -3134,13 +3080,13 @@ int Document::recompute(const std::vector& objs, An alternative to this method might be: https://en.wikipedia.org/wiki/Tarjan%E2%80%99s_strongly_connected_components_algorithm */ -std::vector -DocumentP::partialTopologicalSort(const std::vector& objects) +std::vector +DocumentP::partialTopologicalSort(const std::vector& objects) { - vector ret; + vector ret; ret.reserve(objects.size()); // pairs of input and output degree - map> countMap; + map> countMap; for (auto objectIt : objects) { // we need inlist with unique entries @@ -3156,8 +3102,8 @@ DocumentP::partialTopologicalSort(const std::vector& objec countMap[objectIt] = std::make_pair(in.size(), out.size()); } - std::list degIn; - std::list degOut; + std::list degIn; + std::list degOut; bool removeVertex = true; while (removeVertex) { @@ -3166,7 +3112,7 @@ DocumentP::partialTopologicalSort(const std::vector& objec // try input degree auto degInIt = find_if(countMap.begin(), countMap.end(), - [](pair> vertex) -> bool { + [](pair> vertex) -> bool { return vertex.second.first == 0; }); @@ -3191,9 +3137,9 @@ DocumentP::partialTopologicalSort(const std::vector& objec // make the output degree negative if input degree is negative // to mark the vertex as processed - for (auto& countIt : countMap) { - if (countIt.second.first < 0) { - countIt.second.second = -1; + for (auto& [obj, pair] : countMap) { + if (pair.first < 0) { + pair.second = -1; } } @@ -3203,7 +3149,7 @@ DocumentP::partialTopologicalSort(const std::vector& objec auto degOutIt = find_if(countMap.begin(), countMap.end(), - [](pair> vertex) -> bool { + [](pair> vertex) -> bool { return vertex.second.second == 0; }); @@ -3239,14 +3185,14 @@ DocumentP::partialTopologicalSort(const std::vector& objec return ret; } -std::vector -DocumentP::topologicalSort(const std::vector& objects) const +std::vector +DocumentP::topologicalSort(const std::vector& objects) const { // topological sort algorithm described here: // https://de.wikipedia.org/wiki/Topologische_Sortierung#Algorithmus_f.C3.BCr_das_Topologische_Sortieren - vector ret; + vector ret; ret.reserve(objects.size()); - map countMap; + map countMap; for (auto objectIt : objects) { // We now support externally linked objects @@ -3264,12 +3210,12 @@ DocumentP::topologicalSort(const std::vector& objects) con auto rootObjeIt = find_if(countMap.begin(), countMap.end(), - [](pair count) -> bool { + [](pair count) -> bool { return count.second == 0; }); if (rootObjeIt == countMap.end()) { - cerr << "Document::topologicalSort: cyclic dependency detected (no root object)" << endl; + cerr << "Document::topologicalSort: cyclic dependency detected (no root object)" << '\n'; return ret; } @@ -3291,7 +3237,7 @@ DocumentP::topologicalSort(const std::vector& objects) con rootObjeIt = find_if(countMap.begin(), countMap.end(), - [](pair count) -> bool { + [](pair count) -> bool { return count.second == 0; }); } @@ -3299,18 +3245,18 @@ DocumentP::topologicalSort(const std::vector& objects) con return ret; } -std::vector Document::topologicalSort() const +std::vector Document::topologicalSort() const { return d->topologicalSort(d->objectArray); } -const char* Document::getErrorDescription(const App::DocumentObject* Obj) const +const char* Document::getErrorDescription(const DocumentObject* Obj) const { return d->findRecomputeLog(Obj); } // call the recompute of the Feature and handle the exceptions and errors. -int Document::_recomputeFeature(DocumentObject* Feat) +int Document::_recomputeFeature(DocumentObject* Feat) // NOLINT { FC_LOG("Recomputing " << Feat->getFullName()); @@ -3382,21 +3328,19 @@ bool Document::recomputeFeature(DocumentObject* feature, bool recursive) recompute({feature}, true, &hasError); return !hasError; } - else { - _recomputeFeature(feature); - signalRecomputedObject(*feature); - return feature->isValid(); - } + _recomputeFeature(feature); + signalRecomputedObject(*feature); + return feature->isValid(); } DocumentObject* Document::addObject(const char* sType, const char* pObjectName, - bool isNew, + const bool isNew, const char* viewType, - bool isPartial) + const bool isPartial) { const Base::Type type = - Base::Type::getTypeIfDerivedFrom(sType, App::DocumentObject::getClassTypeId(), true); + Base::Type::getTypeIfDerivedFrom(sType, DocumentObject::getClassTypeId(), true); if (type.isBad()) { std::stringstream str; str << "'" << sType << "' is not a document object type"; @@ -3408,7 +3352,7 @@ DocumentObject* Document::addObject(const char* sType, return nullptr; } - auto* pcObject = static_cast(typeInstance); + auto* pcObject = static_cast(typeInstance); pcObject->setDocument(this); // do no transactions if we do a rollback! @@ -3481,7 +3425,7 @@ std::vector Document::addObjects(const char* sType, const std::vector& objectNames, bool isNew) { Base::Type type = - Base::Type::getTypeIfDerivedFrom(sType, App::DocumentObject::getClassTypeId(), true); + Base::Type::getTypeIfDerivedFrom(sType, DocumentObject::getClassTypeId(), true); if (type.isBad()) { std::stringstream str; str << "'" << sType << "' is not a document object type"; @@ -3491,7 +3435,7 @@ Document::addObjects(const char* sType, const std::vector& objectNa std::vector objects; objects.resize(objectNames.size()); std::generate(objects.begin(), objects.end(), [&] { - return static_cast(type.createInstance()); + return static_cast(type.createInstance()); }); // the type instance could be a null pointer, it is enough to check the first element if (!objects.empty() && !objects[0]) { @@ -3501,7 +3445,7 @@ Document::addObjects(const char* sType, const std::vector& objectNa for (auto it = objects.begin(); it != objects.end(); ++it) { auto index = std::distance(objects.begin(), it); - App::DocumentObject* pcObject = *it; + DocumentObject* pcObject = *it; pcObject->setDocument(this); // do no transactions if we do a rollback! @@ -3598,7 +3542,7 @@ void Document::addObject(DocumentObject* pcObject, const char* pObjectName) d->objectMap[ObjectName] = pcObject; d->objectNameManager.addExactName(ObjectName); // generate object id and add to id map; - if (!pcObject->_Id) { + if (pcObject->_Id == 0) { pcObject->_Id = ++d->lastObjectId; } d->objectIdMap[pcObject->_Id] = pcObject; @@ -3630,11 +3574,11 @@ void Document::addObject(DocumentObject* pcObject, const char* pObjectName) void Document::_addObject(DocumentObject* pcObject, const char* pObjectName) { - std::string ObjectName = getUniqueObjectName(pObjectName); + const std::string ObjectName = getUniqueObjectName(pObjectName); d->objectMap[ObjectName] = pcObject; d->objectNameManager.addExactName(ObjectName); // generate object id and add to id map; - if (!pcObject->_Id) { + if (pcObject->_Id == 0) { pcObject->_Id = ++d->lastObjectId; } d->objectIdMap[pcObject->_Id] = pcObject; @@ -3717,7 +3661,7 @@ void Document::removeObject(const char* sName) } else { // if not saved in undo -> delete object - signalTransactionRemove(*pos->second, 0); + signalTransactionRemove(*pos->second, nullptr); } // Before deleting we must nullify all dependent objects @@ -3751,7 +3695,7 @@ void Document::removeObject(const char* sName) } } - for (std::vector::iterator obj = d->objectArray.begin(); + for (auto obj = d->objectArray.begin(); obj != d->objectArray.end(); ++obj) { if (*obj == pos->second) { @@ -3796,7 +3740,7 @@ void Document::_removeObject(DocumentObject* pcObject) if (sub[sub.size() - 1] != '.') { sub += '.'; } - auto sobj = pos->second->getSubObject(sub.c_str()); + const auto sobj = pos->second->getSubObject(sub.c_str()); if (sobj && sobj->getDocument() == this && !sobj->Visibility.getValue()) { d->activeUndoTransaction->addObjectChange(sobj, &sobj->Visibility); } @@ -3830,7 +3774,7 @@ void Document::_removeObject(DocumentObject* pcObject) } else { // for a rollback delete the object - signalTransactionRemove(*pcObject, 0); + signalTransactionRemove(*pcObject, nullptr); breakDependency(pcObject, true); } // TODO: Transaction::addObjectName could potentially have freed (deleted) pcObject so some of the following @@ -3846,7 +3790,7 @@ void Document::_removeObject(DocumentObject* pcObject) unregisterLabel(pos->second->Label.getStrValue()); d->objectMap.erase(pos); - for (std::vector::iterator it = d->objectArray.begin(); + for (auto it = d->objectArray.begin(); it != d->objectArray.end(); ++it) { if (*it == pcObject) { @@ -3862,7 +3806,7 @@ void Document::_removeObject(DocumentObject* pcObject) } } -void Document::breakDependency(DocumentObject* pcObject, bool clear) +void Document::breakDependency(DocumentObject* pcObject, const bool clear) // NOLINT { // Nullify all dependent objects PropertyLinkBase::breakLinks(pcObject, d->objectArray, clear); @@ -3903,7 +3847,7 @@ Document::copyObject(const std::vector& objs, bool recursive, b use_buffer = false; } - std::vector imported; + std::vector imported; if (use_buffer) { Base::ByteArrayOStreambuf obuf(res); std::ostream ostr(&obuf); @@ -3915,7 +3859,7 @@ Document::copyObject(const std::vector& objs, bool recursive, b imported = md.importObjects(istr); } else { - static Base::FileInfo fi(App::Application::getTempFileName()); + static Base::FileInfo fi(Application::getTempFileName()); Base::ofstream ostr(fi, std::ios::out | std::ios::binary); exportObjects(deps, ostr); ostr.close(); @@ -3928,12 +3872,12 @@ Document::copyObject(const std::vector& objs, bool recursive, b return imported; } - std::unordered_map indices; + std::unordered_map indices; size_t i = 0; for (auto o : deps) { indices[o] = i++; } - std::vector result; + std::vector result; result.reserve(objs.size()); for (auto o : objs) { result.push_back(imported[indices[o]]); @@ -3941,52 +3885,52 @@ Document::copyObject(const std::vector& objs, bool recursive, b return result; } -std::vector -Document::importLinks(const std::vector& objArray) +std::vector +Document::importLinks(const std::vector& objs) { - std::set links; - getLinksTo(links, nullptr, GetLinkExternal, 0, objArray); + std::set links; + getLinksTo(links, nullptr, GetLinkExternal, 0, objs); - std::vector objs; - objs.insert(objs.end(), links.begin(), links.end()); - objs = App::Document::getDependencyList(objs); - if (objs.empty()) { + std::vector vecObjs; + vecObjs.insert(vecObjs.end(), links.begin(), links.end()); + std::vector depObjs = getDependencyList(vecObjs); + if (depObjs.empty()) { FC_ERR("nothing to import"); - return objs; + return depObjs; } - for (auto it = objs.begin(); it != objs.end();) { + for (auto it = depObjs.begin(); it != depObjs.end();) { auto obj = *it; if (obj->getDocument() == this) { - it = objs.erase(it); + it = depObjs.erase(it); continue; } ++it; - if (obj->testStatus(App::PartialObject)) { + if (obj->testStatus(PartialObject)) { throw Base::RuntimeError( "Cannot import partial loaded object. Please reload the current document"); } } - Base::FileInfo fi(App::Application::getTempFileName()); + Base::FileInfo fi(Application::getTempFileName()); { // save stuff to temp file Base::ofstream str(fi, std::ios::out | std::ios::binary); MergeDocuments mimeView(this); - exportObjects(objs, str); + exportObjects(depObjs, str); str.close(); } Base::ifstream str(fi, std::ios::in | std::ios::binary); MergeDocuments mimeView(this); - objs = mimeView.importObjects(str); + depObjs = mimeView.importObjects(str); str.close(); fi.deleteFile(); const auto& nameMap = mimeView.getNameMap(); // First, find all link type properties that needs to be changed - std::map> propMap; - std::vector propList; + std::map> propMap; + std::vector propList; for (auto obj : links) { propList.clear(); obj->getPropertyList(propList); @@ -4010,10 +3954,10 @@ Document::importLinks(const std::vector& objArray) v.first->Paste(*v.second); } - return objs; + return depObjs; } -DocumentObject* Document::moveObject(DocumentObject* obj, bool recursive) +DocumentObject* Document::moveObject(DocumentObject* obj, const bool recursive) { if (!obj) { return nullptr; @@ -4025,17 +3969,17 @@ DocumentObject* Document::moveObject(DocumentObject* obj, bool recursive) // True object move without copy is only safe when undo is off on both // documents. - if (!recursive && !d->iUndoMode && !that->d->iUndoMode && !that->d->rollback) { + if (!recursive && (d->iUndoMode == 0) && (that->d->iUndoMode == 0) && !that->d->rollback) { // all object of the other document that refer to this object must be nullified that->breakDependency(obj, false); - std::string objname = getUniqueObjectName(obj->getNameInDocument()); + const std::string objname = getUniqueObjectName(obj->getNameInDocument()); that->_removeObject(obj); this->_addObject(obj, objname.c_str()); obj->setDocument(this); return obj; } - std::vector deps; + std::vector deps; if (recursive) { deps = getDependencyList({obj}, DepNoXLinked | DepSort); } @@ -4043,7 +3987,7 @@ DocumentObject* Document::moveObject(DocumentObject* obj, bool recursive) deps.push_back(obj); } - auto objs = copyObject(deps, false); + const auto objs = copyObject(deps, false); if (objs.empty()) { return nullptr; } @@ -4051,15 +3995,15 @@ DocumentObject* Document::moveObject(DocumentObject* obj, bool recursive) // or all depending objects for safety reason. std::vector ids; ids.reserve(deps.size()); - for (auto o : deps) { - ids.push_back(o->getID()); + for (const auto o : deps) { + ids.push_back(static_cast(o->getID())); } // We only remove object if it is the moving object or it has no // depending objects, i.e. an empty inList, which is why we need to // iterate the depending list backwards. for (auto iter = ids.rbegin(); iter != ids.rend(); ++iter) { - auto o = that->getObjectByID(*iter); + const auto o = that->getObjectByID(*iter); if (!o) { continue; } @@ -4077,31 +4021,24 @@ DocumentObject* Document::getActiveObject() const DocumentObject* Document::getObject(const char* Name) const { - auto pos = d->objectMap.find(Name); + const auto pos = d->objectMap.find(Name); - if (pos != d->objectMap.end()) { - return pos->second; - } - else { - return nullptr; - } + return pos != d->objectMap.end() ?pos->second:nullptr; } -DocumentObject* Document::getObjectByID(long id) const +DocumentObject* Document::getObjectByID(const long id) const { - auto it = d->objectIdMap.find(id); - if (it != d->objectIdMap.end()) { - return it->second; - } - return nullptr; + const auto it = d->objectIdMap.find(id); + + return it != d->objectIdMap.end() ?it->second:nullptr; } // Note: This method is only used in Tree.cpp slotChangeObject(), see explanation there bool Document::isIn(const DocumentObject* pFeat) const { - for (const auto& pos : d->objectMap) { - if (pos.second == pFeat) { + for (const auto& [key, object] : d->objectMap) { + if (object == pFeat) { return true; } } @@ -4109,11 +4046,11 @@ bool Document::isIn(const DocumentObject* pFeat) const return false; } -const char* Document::getObjectName(DocumentObject* pFeat) const +const char* Document::getObjectName(const DocumentObject* pFeat) const { - for (const auto& pos : d->objectMap) { - if (pos.second == pFeat) { - return pos.first.c_str(); + for (const auto& [key, object] : d->objectMap) { + if (object == pFeat) { + return key.c_str(); } } @@ -4184,7 +4121,7 @@ std::vector Document::getObjectsOfType(const std::vector Document::getObjectsWithExtension(const Base::Type& typeId, - bool derived) const + const bool derived) const { std::vector Objects; @@ -4201,7 +4138,8 @@ std::vector Document::findObjects(const Base::Type& typeId, const char* objname, const char* label) const { boost::cmatch what; - boost::regex rx_name, rx_label; + boost::regex rx_name; + boost::regex rx_label; if (objname) { rx_name.set_expression(objname); @@ -4213,7 +4151,7 @@ Document::findObjects(const Base::Type& typeId, const char* objname, const char* std::vector Objects; DocumentObject* found = nullptr; - for (auto it : d->objectArray) { + for (const auto it : d->objectArray) { if (it->isDerivedFrom(typeId)) { found = it; @@ -4242,7 +4180,7 @@ int Document::countObjectsOfType(const Base::Type& typeId) const int Document::countObjectsOfType(const char* typeName) const { - Base::Type type = Base::Type::fromName(typeName); + const Base::Type type = Base::Type::fromName(typeName); return type.isBad() ? 0 : countObjectsOfType(type); } @@ -4251,9 +4189,9 @@ PyObject* Document::getPyObject() return Py::new_reference_to(d->DocumentPythonObject); } -std::vector Document::getRootObjects() const +std::vector Document::getRootObjects() const { - std::vector ret; + std::vector ret; for (auto objectIt : d->objectArray) { if (objectIt->getInList().empty()) { @@ -4264,9 +4202,9 @@ std::vector Document::getRootObjects() const return ret; } -std::vector Document::getRootObjectsIgnoreLinks() const +std::vector Document::getRootObjectsIgnoreLinks() const { - std::vector ret; + std::vector ret; for (const auto &objectIt : d->objectArray) { auto list = objectIt->getInList(); @@ -4277,8 +4215,8 @@ std::vector Document::getRootObjectsIgnoreLinks() const // So if an object is referenced by an App::Link, it will not be returned by that // function. So here, as we want the tree-root level objects, we check if all the // parents are links. In which case it's still a root object. - noParents = std::all_of(list.cbegin(), list.cend(), [](App::DocumentObject* obj) { - return obj->isDerivedFrom(); + noParents = std::all_of(list.cbegin(), list.cend(), [](DocumentObject* obj) { + return obj->isDerivedFrom(); }); } @@ -4291,7 +4229,7 @@ std::vector Document::getRootObjectsIgnoreLinks() const } void DocumentP::findAllPathsAt(const std::vector& all_nodes, - size_t id, + const size_t id, std::vector& all_paths, Path tmp) { @@ -4308,12 +4246,13 @@ void DocumentP::findAllPathsAt(const std::vector& all_nodes, } for (size_t i = 0; i < all_nodes[id].size(); i++) { - findAllPathsAt(all_nodes, all_nodes[id][i], all_paths, tmp); + const Path& tmp2(tmp); + findAllPathsAt(all_nodes, all_nodes[id][i], all_paths, tmp2); } } -std::vector> -Document::getPathsByOutList(const App::DocumentObject* from, const App::DocumentObject* to) const +std::vector> +Document::getPathsByOutList(const DocumentObject* from, const DocumentObject* to) const { std::map indexMap; for (size_t i = 0; i < d->objectArray.size(); ++i) { @@ -4322,14 +4261,14 @@ Document::getPathsByOutList(const App::DocumentObject* from, const App::Document std::vector all_nodes(d->objectArray.size()); for (size_t i = 0; i < d->objectArray.size(); ++i) { - DocumentObject* obj = d->objectArray[i]; + const DocumentObject* obj = d->objectArray[i]; std::vector outList = obj->getOutList(); - for (auto it : outList) { + for (const auto it : outList) { all_nodes[i].push_back(indexMap[it]); } } - std::vector> array; + std::vector> array; if (from == to) { return array; } @@ -4363,11 +4302,11 @@ bool Document::mustExecute() const { if (PropertyXLink::hasXLink(this)) { bool touched = false; - _buildDependencyList(d->objectArray, false, nullptr, nullptr, nullptr, &touched); + buildDependencyList(d->objectArray, 0, nullptr, nullptr, nullptr, &touched); return touched; } - for (auto It : d->objectArray) { + for (const auto It : d->objectArray) { if (It->isTouched() || It->mustExecute() == 1) { return true; } diff --git a/src/App/Document.h b/src/App/Document.h index 1d4bfb727d..b78baa23f0 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -38,7 +38,6 @@ #include #include #include -#include namespace Base { @@ -62,7 +61,7 @@ using StringHasherRef = Base::Reference; * @ingroup DocumentGroup * @details For a more high-level discussion see the topic @ref DocumentGroup "Document". */ -class AppExport Document: public App::PropertyContainer +class AppExport Document: public PropertyContainer { PROPERTY_HEADER_WITH_OVERRIDE(App::Document); @@ -112,13 +111,13 @@ public: /// unique identifier of the document PropertyUUID Uid; /// Full name of the licence e.g. "Creative Commons Attribution". See https://spdx.org/licenses/ - App::PropertyString License; + PropertyString License; /// License description/contract URL - App::PropertyString LicenseURL; + PropertyString LicenseURL; /// Meta descriptions - App::PropertyMap Meta; + PropertyMap Meta; /// Material descriptions, used and defined in the Material module. - App::PropertyMap Material; + PropertyMap Material; /// read-only name of the temp dir created when the document is opened PropertyString TransientDir; /// Tip object of the document (if any) @@ -135,31 +134,31 @@ public: //@{ // clang-format off /// signal before changing an doc property - boost::signals2::signal signalBeforeChange; + boost::signals2::signal signalBeforeChange; /// signal on changed doc property - boost::signals2::signal signalChanged; + boost::signals2::signal signalChanged; /// signal on new Object - boost::signals2::signal signalNewObject; + boost::signals2::signal signalNewObject; /// signal on deleted Object - boost::signals2::signal signalDeletedObject; + boost::signals2::signal signalDeletedObject; /// signal before changing an Object - boost::signals2::signal signalBeforeChangeObject; + boost::signals2::signal signalBeforeChangeObject; /// signal on changed Object - boost::signals2::signal signalChangedObject; + boost::signals2::signal signalChangedObject; /// signal on manually called DocumentObject::touch() - boost::signals2::signal signalTouchedObject; + boost::signals2::signal signalTouchedObject; /// signal on relabeled Object - boost::signals2::signal signalRelabelObject; + boost::signals2::signal signalRelabelObject; /// signal on activated Object - boost::signals2::signal signalActivatedObject; + boost::signals2::signal signalActivatedObject; /// signal on created object - boost::signals2::signal signalTransactionAppend; + boost::signals2::signal signalTransactionAppend; /// signal on removed object - boost::signals2::signal signalTransactionRemove; + boost::signals2::signal signalTransactionRemove; /// signal on undo - boost::signals2::signal signalUndo; + boost::signals2::signal signalUndo; /// signal on redo - boost::signals2::signal signalRedo; + boost::signals2::signal signalRedo; /** signal on load/save document * this signal is given when the document gets streamed. * you can use this hook to write additional information in @@ -167,28 +166,28 @@ public: */ boost::signals2::signal signalSaveDocument; boost::signals2::signal signalRestoreDocument; - boost::signals2::signal&, Base::Writer&)> signalExportObjects; - boost::signals2::signal&, Base::Writer&)> signalExportViewObjects; - boost::signals2::signal&, Base::XMLReader&)> signalImportObjects; - boost::signals2::signal&, Base::Reader&, + boost::signals2::signal&, Base::Writer&)> signalExportObjects; + boost::signals2::signal&, Base::Writer&)> signalExportViewObjects; + boost::signals2::signal&, Base::XMLReader&)> signalImportObjects; + boost::signals2::signal&, Base::Reader&, const std::map&)> signalImportViewObjects; - boost::signals2::signal&)> signalFinishImportObjects; + boost::signals2::signal&)> signalFinishImportObjects; // signal starting a save action to a file - boost::signals2::signal signalStartSave; + boost::signals2::signal signalStartSave; // signal finishing a save action to a file - boost::signals2::signal signalFinishSave; - boost::signals2::signal signalBeforeRecompute; - boost::signals2::signal&)> signalRecomputed; - boost::signals2::signal signalRecomputedObject; + boost::signals2::signal signalFinishSave; + boost::signals2::signal signalBeforeRecompute; + boost::signals2::signal&)> signalRecomputed; + boost::signals2::signal signalRecomputedObject; // signal a new opened transaction - boost::signals2::signal signalOpenTransaction; + boost::signals2::signal signalOpenTransaction; // signal a committed transaction - boost::signals2::signal signalCommitTransaction; + boost::signals2::signal signalCommitTransaction; // signal an aborted transaction - boost::signals2::signal signalAbortTransaction; - boost::signals2::signal&)> signalSkipRecompute; - boost::signals2::signal signalFinishRestoreObject; - boost::signals2::signal signalChangePropertyEditor; + boost::signals2::signal signalAbortTransaction; + boost::signals2::signal&)> signalSkipRecompute; + boost::signals2::signal signalFinishRestoreObject; + boost::signals2::signal signalChangePropertyEditor; boost::signals2::signal signalLinkXsetValue; // clang-format on //@} @@ -210,16 +209,16 @@ public: bool delaySignal = false, const std::vector& objNames = {}); bool afterRestore(bool checkPartial = false); - bool afterRestore(const std::vector&, bool checkPartial = false); + bool afterRestore(const std::vector&, bool checkPartial = false); enum ExportStatus { NotExporting, Exporting, }; - ExportStatus isExporting(const App::DocumentObject* obj) const; - void exportObjects(const std::vector&, std::ostream&); + ExportStatus isExporting(const DocumentObject* obj) const; + void exportObjects(const std::vector&, std::ostream&); void exportGraphviz(std::ostream&) const; - std::vector importObjects(Base::XMLReader& reader); + std::vector importObjects(Base::XMLReader& reader); /** Import any externally linked objects * * @param objs: input list of objects. Only objects belonging to this document will @@ -232,8 +231,8 @@ public: * * @return the list of imported objects */ - std::vector - importLinks(const std::vector& objs = {}); + std::vector + importLinks(const std::vector& objs = {}); /// Opens the document from its file name // void open (void); /// Is the document already saved to a file? @@ -317,6 +316,7 @@ public: /** Copy objects from another document to this document * + * @param objs * @param recursive: if true, then all objects this object depends on are * copied as well. By default \a recursive is false. * @@ -345,7 +345,7 @@ public: /// Returns true if the DocumentObject is contained in this document bool isIn(const DocumentObject* pFeat) const; /// Returns a Name of an Object or 0 - const char *getObjectName(DocumentObject* pFeat) const; + const char *getObjectName(const DocumentObject* pFeat) const; /// Returns a Name for a new Object or empty if proposedName is null or empty. std::string getUniqueObjectName(const char* proposedName) const; /// Returns a name different from any of the Labels of any objects in this document, based on the given modelName. @@ -384,7 +384,7 @@ public: /// check if there is any object must execute in this document bool mustExecute() const; /// returns all touched objects - std::vector getTouched() const; + std::vector getTouched() const; /// set the document to be closable, this is on by default. void setClosable(bool); /// check whether the document can be closed @@ -397,15 +397,18 @@ public: * * @param objs: specify a sub set of objects to recompute. If empty, then * all object in this document is checked for recompute + * @param force + * @param hasError + * @param options */ - int recompute(const std::vector& objs = {}, + int recompute(const std::vector& objs = {}, bool force = false, bool* hasError = nullptr, int options = 0); /// Recompute only one feature bool recomputeFeature(DocumentObject* Feat, bool recursive = false); /// get the text of the error of a specified object - const char* getErrorDescription(const App::DocumentObject*) const; + const char* getErrorDescription(const DocumentObject*) const; /// return the status bits bool testStatus(Status pos) const; /// set the status bits @@ -440,17 +443,17 @@ public: * * @param name: transaction name * - * This function calls App::Application::setActiveTransaction(name) instead + * This function calls Application::setActiveTransaction(name) instead * to setup a potential transaction which will only be created if there is * actual changes. */ void openTransaction(const char* name = nullptr); /// Rename the current transaction if the id matches - void renameTransaction(const char* name, int id); + void renameTransaction(const char* name, int id) const; /// Commit the Command transaction. Do nothing If there is no Command transaction open. void commitTransaction(); /// Abort the actually running transaction. - void abortTransaction(); + void abortTransaction() const; /// Check if a transaction is open bool hasPendingTransaction() const; /// Return the undo/redo transaction ID starting from the back @@ -484,7 +487,7 @@ public: /// redo/undo or rollback bool isPerformingTransaction() const; /// \internal add or remove property from a transactional object - void addOrRemovePropertyOfObject(TransactionalObject*, Property* prop, bool add); + void addOrRemovePropertyOfObject(TransactionalObject*, const Property* prop, bool add); //@} /** @name dependency stuff */ @@ -492,9 +495,9 @@ public: /// write GraphViz file void writeDependencyGraphViz(std::ostream& out); /// checks if the graph is directed and has no cycles - bool checkOnCycle(); + static bool checkOnCycle(); /// get a list of all objects linking to the given object - std::vector getInList(const DocumentObject* me) const; + std::vector getInList(const DocumentObject* me) const; /// Option bit flags used by getDepenencyList() enum DependencyOption @@ -515,24 +518,24 @@ public: * @param objs: input objects to query for dependency. * @param options: See DependencyOption */ - static std::vector - getDependencyList(const std::vector& objs, int options = 0); + static std::vector + getDependencyList(const std::vector& objs, int options = 0); - std::vector getDependentDocuments(bool sort = true); - static std::vector getDependentDocuments(std::vector docs, + std::vector getDependentDocuments(bool sort = true); + static std::vector getDependentDocuments(std::vector docs, bool sort); // set Changed // void setChanged(DocumentObject* change); /// get a list of topological sorted objects (https://en.wikipedia.org/wiki/Topological_sorting) - std::vector topologicalSort() const; + std::vector topologicalSort() const; /// get all root objects (objects no other one reference too) - std::vector getRootObjects() const; + std::vector getRootObjects() const; /// get all tree root objects (objects that are at the root of the object tree) - std::vector getRootObjectsIgnoreLinks() const; + std::vector getRootObjectsIgnoreLinks() const; /// get all possible paths from one object to another following the OutList - std::vector> - getPathsByOutList(const App::DocumentObject* from, const App::DocumentObject* to) const; + std::vector> + getPathsByOutList(const DocumentObject* from, const DocumentObject* to) const; //@} /** Called by a property during save to store its StringHasher @@ -566,7 +569,7 @@ public: * * @param links: holds the links found * @param obj: the linked object. If NULL, then all links are returned. - * @param option: @sa App::GetLinkOption + * @param options: @sa GetLinkOption * @param maxCount: limit the number of links returned, 0 means no limit * @param objs: optional objects to search for, if empty, then all objects * of this document are searched. @@ -590,9 +593,9 @@ public: /// Function called to signal that an object identifier has been renamed void renameObjectIdentifiers( - const std::map& paths, - const std::function& selector = - [](const App::DocumentObject*) { + const std::map& paths, + const std::function& selector = + [](const DocumentObject*) { return true; }); @@ -627,8 +630,8 @@ protected: /// checks if a valid transaction is open void _checkTransaction(DocumentObject* pcDelObj, const Property* What, int line); void breakDependency(DocumentObject* pcObject, bool clear); - std::vector readObjects(Base::XMLReader& reader); - void writeObjects(const std::vector&, Base::Writer& writer) const; + std::vector readObjects(Base::XMLReader& reader); + void writeObjects(const std::vector&, Base::Writer& writer) const; bool saveToFile(const char* filename) const; int countObjectsOfType(const Base::Type& typeId) const; @@ -645,7 +648,7 @@ protected: /// refresh the internal dependency graph void _rebuildDependencyList( - const std::vector& objs = std::vector()); + const std::vector& objs = std::vector()); std::string getTransientDirectoryName(const std::string& uuid, const std::string& filename) const; @@ -661,9 +664,9 @@ protected: * AutoTransaction setting. */ int _openTransaction(const char* name = nullptr, int id = 0); - /// Internally called by App::Application to commit the Command transaction. + /// Internally called by Application to commit the Command transaction. void _commitTransaction(bool notify = false); - /// Internally called by App::Application to abort the running transaction. + /// Internally called by Application to abort the running transaction. void _abortTransaction(); private: @@ -685,7 +688,7 @@ template inline std::vector Document::getObjectsOfType() const { std::vector type; - std::vector obj = this->getObjectsOfType(T::getClassTypeId()); + const std::vector obj = this->getObjectsOfType(T::getClassTypeId()); type.reserve(obj.size()); for (auto it : obj) { type.push_back(static_cast(it)); @@ -696,7 +699,7 @@ inline std::vector Document::getObjectsOfType() const template inline int Document::countObjectsOfType() const { - static_assert(std::is_base_of::value, + static_assert(std::is_base_of_v, "T must be derived from App::DocumentObject"); return this->countObjectsOfType(T::getClassTypeId()); } diff --git a/src/App/private/DocumentP.h b/src/App/private/DocumentP.h index ae89b1b206..eac25f6633 100644 --- a/src/App/private/DocumentP.h +++ b/src/App/private/DocumentP.h @@ -77,26 +77,26 @@ struct DocumentP std::unordered_map objectIdMap; std::unordered_map partialLoadObjects; std::vector pendingRemove; - long lastObjectId; - DocumentObject* activeObject; - Transaction* activeUndoTransaction; + long lastObjectId {}; + DocumentObject* activeObject {nullptr}; + Transaction* activeUndoTransaction {nullptr}; // pointer to the python class Py::Object DocumentPythonObject; - int iTransactionMode; - bool rollback; - bool undoing; ///< document in the middle of undo or redo - bool committing; - bool opentransaction; + int iTransactionMode {0}; + bool rollback {false}; + bool undoing {false}; ///< document in the middle of undo or redo + bool committing {false}; + bool opentransaction {false}; std::bitset<32> StatusBits; - int iUndoMode; - unsigned int UndoMemSize; - unsigned int UndoMaxStackSize; + int iUndoMode {0}; + unsigned int UndoMemSize {0}; + unsigned int UndoMaxStackSize {20}; std::string programVersion; mutable HasherMap hashers; std::multimap> _RecomputeLog; - StringHasherRef Hasher; + StringHasherRef Hasher {new StringHasher}; DocumentP();