Use "return object" instead of return parameters

In comparison to alternative approaches like tuples, this allows naming the return parameters, which makes them easier to use for callers.
owncloud · Apr 13, 2021 · 96c446d · 96c446d
1 parent 6e35c8f
commit 96c446d
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 33 deletions.
diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp
@@ -1230,12 +1230,9 @@ void FolderMan::setDirtyNetworkLimits()
     }
 }
 
-void FolderMan::trayOverallStatus(const QList<Folder *> &folders, SyncResult::Status &status, bool &unresolvedConflicts, QDateTime &lastSyncDone)
+TrayOverallStatusResult FolderMan::trayOverallStatus(const QList<Folder *> &folders)
 {
-    status = SyncResult::Undefined;
-    unresolvedConflicts = false;
-    // initialize with a "null" value (i.e., isNull() returns true)
-    lastSyncDone = QDateTime();
+    TrayOverallStatusResult result;
 
     int cnt = folders.count();
 
@@ -1250,31 +1247,31 @@ void FolderMan::trayOverallStatus(const QList<Folder *> &folders, SyncResult::St
         if (folder) {
             auto syncResult = folder->syncResult();
             if (folder->syncPaused()) {
-                status = SyncResult::Paused;
+                result.overallStatus = SyncResult::Paused;
             } else {
                 SyncResult::Status syncStatus = syncResult.status();
                 switch (syncStatus) {
                 case SyncResult::Undefined:
-                    status = SyncResult::Error;
+                    result.overallStatus = SyncResult::Error;
                     break;
                 case SyncResult::Problem: // don't show the problem icon in tray.
-                    status = SyncResult::Success;
+                    result.overallStatus = SyncResult::Success;
                     break;
                 default:
-                    status = syncStatus;
+                    result.overallStatus = syncStatus;
                     break;
                 }
             }
 
-            unresolvedConflicts = syncResult.hasUnresolvedConflicts();
+            result.hasUnresolvedConflicts = syncResult.hasUnresolvedConflicts();
 
             auto currentFolderLastSyncDone = QDateTime::currentDateTime().addMSecs(-1 * folder->msecSinceLastSync().count());
 
-            if (lastSyncDone.isNull()) {
-                lastSyncDone = currentFolderLastSyncDone;
+            if (result.lastSyncDone.isNull()) {
+                result.lastSyncDone = currentFolderLastSyncDone;
             } else {
-                if (currentFolderLastSyncDone > lastSyncDone) {
-                    lastSyncDone = currentFolderLastSyncDone;
+                if (currentFolderLastSyncDone > result.lastSyncDone) {
+                    result.lastSyncDone = currentFolderLastSyncDone;
                 }
             }
         }
@@ -1316,19 +1313,21 @@ void FolderMan::trayOverallStatus(const QList<Folder *> &folders, SyncResult::St
                 }
             }
             if (folderResult.hasUnresolvedConflicts())
-                unresolvedConflicts = true;
+                result.hasUnresolvedConflicts = true;
         }
         if (errorsSeen > 0) {
-            status = SyncResult::Error;
+            result.overallStatus = SyncResult::Error;
         } else if (abortOrPausedSeen > 0 && abortOrPausedSeen == cnt) {
             // only if all folders are paused
-            status = SyncResult::Paused;
+            result.overallStatus = SyncResult::Paused;
         } else if (runSeen > 0) {
-            status = SyncResult::SyncRunning;
+            result.overallStatus = SyncResult::SyncRunning;
         } else if (goodSeen > 0) {
-            status = SyncResult::Success;
+            result.overallStatus = SyncResult::Success;
         }
     }
+
+    return result;
 }
 
 QString FolderMan::trayTooltipStatusString(

diff --git a/src/gui/folderman.h b/src/gui/folderman.h
@@ -34,6 +34,17 @@ class SyncResult;
 class SocketApi;
 class LockWatcher;
 
+/**
+ * @brief Return object for Folder::trayOverallStatus.
+ * @ingroup gui
+ */
+class TrayOverallStatusResult {
+public:
+    SyncResult::Status overallStatus;
+    bool hasUnresolvedConflicts;
+    QDateTime lastSyncDone;
+};
+
 /**
  * @brief The FolderMan class
  * @ingroup gui
@@ -134,8 +145,11 @@ class FolderMan : public QObject
     /// Produce text for use in the tray tooltip
     static QString trayTooltipStatusString(SyncResult::Status syncStatus, bool hasUnresolvedConflicts, bool paused);
 
-    /// Compute status summarizing multiple folders
-    static void trayOverallStatus(const QList<Folder *> &folders, SyncResult::Status &status, bool &unresolvedConflicts, QDateTime&lastSyncDone);
+    /**
+     * Compute status summarizing multiple folders
+     * @return tuple containing folders, status, unresolvedConflicts and lastSyncDone
+     */
+    static TrayOverallStatusResult trayOverallStatus(const QList<Folder *>& folders);
 
     // Escaping of the alias which is used in QSettings AND the file
     // system, thus need to be escaped.

diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp
@@ -263,15 +263,12 @@ void ownCloudGui::slotComputeOverallSyncStatus()
     FolderMan *folderMan = FolderMan::instance();
     Folder::Map map = folderMan->map();
 
-    SyncResult::Status overallStatus = SyncResult::Undefined;
-    bool hasUnresolvedConflicts = false;
-    QDateTime lastSyncDone;
-    FolderMan::trayOverallStatus(map.values(), overallStatus, hasUnresolvedConflicts, lastSyncDone);
+    auto trayOverallStatusResult = FolderMan::trayOverallStatus(map.values());
 
     // If the sync succeeded but there are unresolved conflicts,
     // show the problem icon!
-    auto iconStatus = overallStatus;
-    if (iconStatus == SyncResult::Success && hasUnresolvedConflicts) {
+    auto iconStatus = trayOverallStatusResult.overallStatus;
+    if (iconStatus == SyncResult::Success && trayOverallStatusResult.hasUnresolvedConflicts) {
         iconStatus = SyncResult::Problem;
     }
 
@@ -301,22 +298,22 @@ void ownCloudGui::slotComputeOverallSyncStatus()
 #endif
         _tray->setToolTip(trayMessage);
 
-        if (overallStatus == SyncResult::Success || overallStatus == SyncResult::Problem) {
-            if (hasUnresolvedConflicts) {
+        if (trayOverallStatusResult.overallStatus == SyncResult::Success || trayOverallStatusResult.overallStatus == SyncResult::Problem) {
+            if (trayOverallStatusResult.hasUnresolvedConflicts) {
                 setStatusText(tr("Unresolved conflicts"));
             } else {
                 QString lastSyncDoneString;
 
                 // display only the time in case the last sync was today
-                if (QDateTime::currentDateTime().date() == lastSyncDone.date()) {
-                    lastSyncDoneString = QLocale().toString(lastSyncDone.time());
+                if (QDateTime::currentDateTime().date() == trayOverallStatusResult.lastSyncDone.date()) {
+                    lastSyncDoneString = QLocale().toString(trayOverallStatusResult.lastSyncDone.time());
                 } else {
-                    lastSyncDoneString = QLocale().toString(lastSyncDone);
+                    lastSyncDoneString = QLocale().toString(trayOverallStatusResult.lastSyncDone);
                 }
 
                 setStatusText(tr("Up to date (%1)").arg(lastSyncDoneString));
             }
-        } else if (overallStatus == SyncResult::Paused) {
+        } else if (trayOverallStatusResult.overallStatus == SyncResult::Paused) {
             setStatusText(tr("Synchronization is paused"));
         } else {
             setStatusText(tr("Error during synchronization"));