From 2c2cdbfe7f94735a377c06f98389f507aed0d8c1 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@freecad.org>
Date: Wed, 12 Mar 2025 16:31:42 +0100
Subject: [PATCH] App: Refactor Document::writeObjects

---
 src/App/Document.cpp | 182 ++++++++++++++++++++++++-------------------
 src/App/Document.h   |   4 +
 2 files changed, 107 insertions(+), 79 deletions(-)

diff --git a/src/App/Document.cpp b/src/App/Document.cpp
index b95e2c21b2..2d2bfa940e 100644
--- a/src/App/Document.cpp
+++ b/src/App/Document.cpp
@@ -93,7 +93,6 @@ using Base::Console;
 using Base::streq;
 using Base::Writer;
 using namespace App;
-using namespace std;
 using namespace boost;
 using namespace zipios;
 
@@ -1251,98 +1250,123 @@ constexpr auto fcAttrDepObjName {"Name"};
 constexpr auto fcAttrDepAllowPartial {"AllowPartial"};
 constexpr auto fcElementObjectDep {"Dep"};
 
-void Document::writeObjects(const std::vector<DocumentObject*>& objs,
-                            Base::Writer& writer) const
+void Document::writeObjectDeps(const std::vector<DocumentObject*>& objs,
+                               Base::Writer& writer) const
 {
-    // writing the features types
-    writer.incInd();  // indentation for 'Objects count'
-    writer.Stream() << writer.ind() << "<Objects Count=\"" << objs.size();
-    if (isExporting(nullptr) == 0U) {
-        writer.Stream() << "\" " << fcAttrDependencies << "=\"1";
-    }
-    writer.Stream() << "\">" << '\n';
+    for (auto o : objs) {
+        // clang-format off
+        const auto& outList = o->getOutList(DocumentObject::OutListNoHidden |
+                                            DocumentObject::OutListNoXLinked);
+        // clang-format on
 
-    writer.incInd();  // indentation for 'Object type'
-
-    if (isExporting(nullptr) == 0U) {
-        for (const auto o : objs) {
-            const auto& outList =
-                o->getOutList(DocumentObject::OutListNoHidden | DocumentObject::OutListNoXLinked);
-            writer.Stream() << writer.ind()
-                            << "<" << fcElementObjectDeps << " " << fcAttrDepObjName << "=\""
-                            << o->getNameInDocument() << "\" " << fcAttrDepCount << "=\""
-                            << outList.size();
-            if (outList.empty()) {
-                writer.Stream() << "\"/>" << '\n';
-                continue;
-            }
-            const int partial = o->canLoadPartial();
-            if (partial > 0) {
-                writer.Stream() << "\" " << fcAttrDepAllowPartial << "=\"" << partial;
-            }
-            writer.Stream() << "\">" << '\n';
-            writer.incInd();
-            for (const auto dep : outList) {
-                const auto name = dep ? dep->getNameInDocument() : "";
-                writer.Stream() << writer.ind()
-                                << "<" << fcElementObjectDep << " " << fcAttrDepObjName << "=\""
-                                << (name ? name : "") << "\"/>" << '\n';
-            }
-            writer.decInd();
-            writer.Stream() << writer.ind() << "</" << fcElementObjectDeps << ">" << '\n';
+        writer.Stream() << writer.ind()
+                        << "<" << fcElementObjectDeps
+                        << " " << fcAttrDepObjName << "=\""
+                        << o->getNameInDocument() << "\" "
+                        << fcAttrDepCount << "=\""
+                        << outList.size();
+        if (outList.empty()) {
+            writer.Stream() << "\"/>\n";
+            continue;
         }
+        int partial = o->canLoadPartial();
+        if (partial > 0) {
+            writer.Stream() << "\" " << fcAttrDepAllowPartial << "=\"" << partial;
+        }
+        writer.Stream() << "\">\n";
+        writer.incInd();
+        for (auto dep : outList) {
+            auto name = dep ? dep->getNameInDocument() : "";
+            writer.Stream() << writer.ind()
+                            << "<" << fcElementObjectDep
+                            << " " << fcAttrDepObjName << "=\""
+                            << (name ? name : "") << "\"/>\n";
+        }
+        writer.decInd();
+        writer.Stream() << writer.ind() << "</" << fcElementObjectDeps << ">\n";
     }
+}
 
-    std::vector<DocumentObject*>::const_iterator it;
-    for (it = objs.begin(); it != objs.end(); ++it) {
+void Document::writeObjectType(const std::vector<DocumentObject*>& objs,
+                               Base::Writer& writer) const
+{
+    for (auto it : objs) {
         writer.Stream() << writer.ind() << "<Object "
-                        << "type=\"" << (*it)->getTypeId().getName() << "\" "
-                        << "name=\"" << (*it)->getExportName() << "\" "
-                        << "id=\"" << (*it)->getID() << "\" ";
+                        << "type=\"" << it->getTypeId().getName() << "\" "
+                        << "name=\"" << it->getExportName() << "\" "
+                        << "id=\"" << it->getID() << "\" ";
 
         // Only write out custom view provider types
-        std::string viewType = (*it)->getViewProviderNameStored();
-        if (viewType != (*it)->getViewProviderName()) {
+        std::string viewType = it->getViewProviderNameStored();
+        if (viewType != it->getViewProviderName()) {
             writer.Stream() << "ViewType=\"" << viewType << "\" ";
         }
 
         // See DocumentObjectPy::getState
-        if ((*it)->testStatus(ObjectStatus::Touch)) {
+        if (it->testStatus(ObjectStatus::Touch)) {
             writer.Stream() << "Touched=\"1\" ";
         }
-        if ((*it)->testStatus(ObjectStatus::Error)) {
+        if (it->testStatus(ObjectStatus::Error)) {
             writer.Stream() << "Invalid=\"1\" ";
-            const auto desc = getErrorDescription(*it);
+            auto desc = getErrorDescription(it);
             if (desc) {
                 writer.Stream() << "Error=\"" << Property::encodeAttribute(desc) << "\" ";
             }
         }
-        if ((*it)->isFreezed()) {
+        if (it->isFreezed()) {
             writer.Stream() << "Freeze=\"1\" ";
         }
-        writer.Stream() << "/>" << '\n';
+        writer.Stream() << "/>\n";
     }
+}
 
-    writer.decInd();  // indentation for 'Object type'
-    writer.Stream() << writer.ind() << "</Objects>" << '\n';
-
+void Document::writeObjectData(const std::vector<DocumentObject*>& objs,
+                               Base::Writer& writer) const
+{
     // writing the features itself
-    writer.Stream() << writer.ind() << "<ObjectData Count=\"" << objs.size() << "\">" << '\n';
+    writer.Stream() << writer.ind() << "<ObjectData Count=\"" << objs.size() << "\">\n";
 
     writer.incInd();  // indentation for 'Object name'
-    for (it = objs.begin(); it != objs.end(); ++it) {
-        writer.Stream() << writer.ind() << "<Object name=\"" << (*it)->getExportName() << "\"";
-        if ((*it)->hasExtensions()) {
+    for (auto it : objs) {
+        writer.Stream() << writer.ind() << "<Object name=\"" << it->getExportName() << "\"";
+        if (it->hasExtensions()) {
             writer.Stream() << " Extensions=\"True\"";
         }
 
-        writer.Stream() << ">" << '\n';
-        (*it)->Save(writer);
-        writer.Stream() << writer.ind() << "</Object>" << '\n';
+        writer.Stream() << ">\n";
+        it->Save(writer);
+        writer.Stream() << writer.ind() << "</Object>\n";
+    }
+    writer.decInd();  // indentation for 'Object name'
+
+    writer.Stream() << writer.ind() << "</ObjectData>\n";
+}
+
+void Document::writeObjects(const std::vector<DocumentObject*>& objs,
+                            Base::Writer& writer) const
+{
+    std::ostream& str = writer.Stream();
+
+    // writing the features types
+    writer.incInd();  // indentation for 'Objects count'
+    str << writer.ind() << "<Objects Count=\"" << objs.size();
+    if (!isExporting(nullptr)) {
+        str << "\" " << fcAttrDependencies << "=\"1";
+    }
+    str << "\">\n";
+
+    writer.incInd();  // indentation for 'Object type'
+
+    if (!isExporting(nullptr)) {
+        writeObjectDeps(objs, writer);
     }
 
-    writer.decInd();  // indentation for 'Object name'
-    writer.Stream() << writer.ind() << "</ObjectData>" << '\n';
+    writeObjectType(objs, writer);
+
+    writer.decInd();  // indentation for 'Object type'
+    str << writer.ind() << "</Objects>\n";
+
+    writeObjectData(objs, writer);
     writer.decInd();  // indentation for 'Objects count'
 }
 
@@ -2824,10 +2848,10 @@ int Document::recompute(const std::vector<DocumentObject*>& objs,
 std::vector<DocumentObject*>
 DocumentP::partialTopologicalSort(const std::vector<DocumentObject*>& objects)
 {
-    vector<DocumentObject*> ret;
+    std::vector<DocumentObject*> ret;
     ret.reserve(objects.size());
     // pairs of input and output degree
-    map<DocumentObject*, std::pair<int, int>> countMap;
+    std::map<DocumentObject*, std::pair<int, int>> countMap;
 
     for (auto objectIt : objects) {
         // we need inlist with unique entries
@@ -2853,7 +2877,7 @@ DocumentP::partialTopologicalSort(const std::vector<DocumentObject*>& objects)
         // try input degree
         auto degInIt = find_if(countMap.begin(),
                                countMap.end(),
-                               [](pair<DocumentObject*, pair<int, int>> vertex) -> bool {
+                               [](std::pair<DocumentObject*, std::pair<int, int>> vertex) -> bool {
                                    return vertex.second.first == 0;
                                });
 
@@ -2888,11 +2912,11 @@ DocumentP::partialTopologicalSort(const std::vector<DocumentObject*>& objects)
     while (removeVertex) {
         removeVertex = false;
 
-        auto degOutIt = find_if(countMap.begin(),
-                                countMap.end(),
-                                [](pair<DocumentObject*, pair<int, int>> vertex) -> bool {
-                                    return vertex.second.second == 0;
-                                });
+        auto degOutIt = std::find_if(countMap.begin(),
+                                     countMap.end(),
+                                     [](std::pair<DocumentObject*, std::pair<int, int>> vertex) -> bool {
+                                         return vertex.second.second == 0;
+                                     });
 
         if (degOutIt != countMap.end()) {
             removeVertex = true;
@@ -2931,9 +2955,9 @@ DocumentP::topologicalSort(const std::vector<DocumentObject*>& objects) const
 {
     // topological sort algorithm described here:
     // https://de.wikipedia.org/wiki/Topologische_Sortierung#Algorithmus_f.C3.BCr_das_Topologische_Sortieren
-    vector<DocumentObject*> ret;
+    std::vector<DocumentObject*> ret;
     ret.reserve(objects.size());
-    map<DocumentObject*, int> countMap;
+    std::map<DocumentObject*, int> countMap;
 
     for (auto objectIt : objects) {
         // We now support externally linked objects
@@ -2949,14 +2973,14 @@ DocumentP::topologicalSort(const std::vector<DocumentObject*>& objects) const
         countMap[objectIt] = in.size();
     }
 
-    auto rootObjeIt = find_if(countMap.begin(),
-                              countMap.end(),
-                              [](pair<DocumentObject*, int> count) -> bool {
-                                  return count.second == 0;
-                              });
+    auto rootObjeIt = std::find_if(countMap.begin(),
+                                   countMap.end(),
+                                   [](std::pair<DocumentObject*, int> count) -> bool {
+                                       return count.second == 0;
+                                   });
 
     if (rootObjeIt == countMap.end()) {
-        cerr << "Document::topologicalSort: cyclic dependency detected (no root object)" << '\n';
+        std::cerr << "Document::topologicalSort: cyclic dependency detected (no root object)" << '\n';
         return ret;
     }
 
@@ -2978,7 +3002,7 @@ DocumentP::topologicalSort(const std::vector<DocumentObject*>& objects) const
 
         rootObjeIt = find_if(countMap.begin(),
                              countMap.end(),
-                             [](pair<DocumentObject*, int> count) -> bool {
+                             [](std::pair<DocumentObject*, int> count) -> bool {
                                  return count.second == 0;
                              });
     }
diff --git a/src/App/Document.h b/src/App/Document.h
index 7347ff7513..8dd59b3023 100644
--- a/src/App/Document.h
+++ b/src/App/Document.h
@@ -1329,6 +1329,10 @@ protected:
      */
     void writeObjects(const std::vector<DocumentObject*>& objs, Base::Writer& writer) const;
 
+    void writeObjectDeps(const std::vector<DocumentObject*>& objs, Base::Writer& writer) const;
+    void writeObjectType(const std::vector<DocumentObject*>& objs, Base::Writer& writer) const;
+    void writeObjectData(const std::vector<DocumentObject*>& objs, Base::Writer& writer) const;
+
     /**
      * @brief Save the document to a file.
      *