Merge lp:~mandel/ubuntu-download-manager/remove-q-pimpl into lp:ubuntu-download-manager

[Manuel de la Peña]

Remove the pimpl pattern from the Queue since it is a class not to be shared outside the project.

Remove the pimpl pattern from the Queue since it is a class not to be shared outside the project.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:203
http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-ci/257/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-trusty-amd64-ci/146
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-trusty-armhf-ci/146

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-download-manager-ci/257/rebuild

review: Approve (continuous-integration)

[Diego Sarmentero (diegosarmentero)]

Looks good.

This only things looks kind of odd to me:
288 + default:
289 + // do nothing
290 + break;
291 + }

But i understand that mmcc already said that was necessary for some reason in a previous branch.
Mike, if you can explain that please, i'll not block the branch for that if you requested it before.

review: Approve

=== modified file 'libubuntudownloadmanager/downloads/queue.cpp'
--- libubuntudownloadmanager/downloads/queue.cpp	2013-11-25 14:46:52 +0000
+++ libubuntudownloadmanager/downloads/queue.cpp	2013-12-12 16:41:39 +0000
@@ -25,216 +25,158 @@
 
 namespace DownloadManager {
 
-/*
- * PRIVATE IMPLEMENTATION
- */
-
-class QueuePrivate {
-    Q_DECLARE_PUBLIC(Queue)
-
- public:
-    QueuePrivate(SystemNetworkInfo* networkInfo,
-                 Queue* parent)
-        : _current(""),
-          _networkInfo(networkInfo),
-          q_ptr(parent) {
-          Q_Q(Queue);
-
-        q->connect(_networkInfo,
-            SIGNAL(currentNetworkModeChanged(QNetworkInfo::NetworkMode)), q,
-            SLOT(onCurrentNetworkModeChanged(QNetworkInfo::NetworkMode)));
-    }
-
-    void add(Download* download) {
-        Q_Q(Queue);
-        // connect to the signals and append to the list
-        QString path = download->path();
-        TRACE << path;
-
-        _sortedPaths.append(path);
-        _downloads[path] = download;
-
-        q->connect(download, SIGNAL(stateChanged()),
-            q, SLOT(onDownloadStateChanged()));
-        emit q->downloadAdded(path);
-    }
-
-    void remove(const QString& path) {
-        Q_Q(Queue);
-        TRACE << path;
-
-        Download* down = _downloads[path];
-        _sortedPaths.removeOne(path);
-        _downloads.remove(path);
-
-        down->deleteLater();
-        emit q->downloadRemoved(path);
-    }
-
-    QString currentDownload() {
-        return _current;
-    }
-
-    QStringList paths() {
-        return _sortedPaths;
-    }
-
-    QHash<QString, Download*> downloads() {
-        QHash<QString, Download*> downloads;
-        foreach(const QString& path, _sortedPaths) {
-            downloads[path] = _downloads[path];
-        }
-        return downloads;
-    }
-
-    int size() {
-        return _downloads.size();
-    }
-
-    void onDownloadStateChanged() {
-        TRACE;
-        Q_Q(Queue);
-        // get the appdownload that emited the signal and
-        // decide what to do with it
-        Download* sender = qobject_cast<Download*>(q->sender());
-        switch (sender->state()) {
-            case Download::START:
-                // only start the download in the update method
-                if (_current.isEmpty())
-                    updateCurrentDownload();
-                break;
-            case Download::PAUSE:
-                sender->pauseDownload();
-                if (!_current.isEmpty()  && _current == sender->path())
-                    updateCurrentDownload();
-                break;
-            case Download::RESUME:
-                // only resume the download in the update method
-                if (_current.isEmpty())
-                    updateCurrentDownload();
-                break;
-            case Download::CANCEL:
-                // cancel and remove the download
-                sender->cancelDownload();
-                if (!_current.isEmpty() && _current == sender->path())
-                    updateCurrentDownload();
-                else
-                    remove(sender->path());
-                break;
-            case Download::ERROR:
-            case Download::FINISH:
-                // remove the registered object in dbus, remove the download
-                // and the adapter from the list
-                if (!_current.isEmpty() && _current == sender->path())
-                    updateCurrentDownload();
-                break;
-            default:
-                // do nothing
-                break;
-        }
-    }
-
-    void onCurrentNetworkModeChanged(QNetworkInfo::NetworkMode mode) {
-        TRACE << mode;
-        if (mode != QNetworkInfo::UnknownMode) {
-            updateCurrentDownload();
-        }
-    }
-
- private:
-    void updateCurrentDownload() {
-        TRACE;
-        Q_Q(Queue);
-
-        if (!_current.isEmpty()) {
-            // check if it was canceled/finished
-            Download* currentDownload = _downloads[_current];
-            Download::State state = currentDownload->state();
-            if (state == Download::CANCEL || state == Download::FINISH
-                || state == Download::ERROR) {
-                qDebug() << "State is CANCEL || FINISH || ERROR";
-                remove(_current);
-                _current = "";
-            } else if (!currentDownload->canDownload()
-                    || state == Download::PAUSE) {
-                qDebug() << "States is Cannot Download || PAUSE";
-                _current = "";
-            } else {
-                return;
-            }
-        }
-
-        // loop via the downloads and choose the first that is
-        // started or resumed
-        foreach(const QString& path, _sortedPaths) {
-            Download* down = _downloads[path];
-            Download::State state = down->state();
-            if (down->canDownload()
-                    && (state == Download::START
-                        || state == Download::RESUME)) {
-                _current = path;
-                if (state == Download::START)
-                    down->startDownload();
-                else
-                    down->resumeDownload();
-                break;
-            }
-        }
-
-        emit q->currentChanged(_current);
-    }
-
- private:
-    QString _current;
-    QHash<QString, Download*> _downloads;  // quick for access
-    QStringList _sortedPaths;  // keep the order
-    SystemNetworkInfo* _networkInfo;
-    Queue* q_ptr;
-};
-
-/*
- * PUBLIC IMPLEMENTATION
- */
-
 Queue::Queue(SystemNetworkInfo* networkInfo,
              QObject* parent)
     : QObject(parent),
-      d_ptr(new QueuePrivate(networkInfo, this)) {
+      _current(""),
+      _networkInfo(networkInfo) {
+    connect(_networkInfo, &SystemNetworkInfo::currentNetworkModeChanged,
+        this, &Queue::onCurrentNetworkModeChanged);
 }
 
 void
 Queue::add(Download* download) {
-    Q_D(Queue);
-    d->add(download);
+    // connect to the signals and append to the list
+    QString path = download->path();
+    TRACE << path;
+
+    _sortedPaths.append(path);
+    _downloads[path] = download;
+
+    connect(download, &Download::stateChanged,
+        this, &Queue::onDownloadStateChanged);
+    emit downloadAdded(path);
+}
+
+void
+Queue::remove(const QString& path) {
+    TRACE << path;
+
+    Download* down = _downloads[path];
+    _sortedPaths.removeOne(path);
+    _downloads.remove(path);
+
+    down->deleteLater();
+    emit downloadRemoved(path);
 }
 
 QString
 Queue::currentDownload() {
-    Q_D(Queue);
-    return d->currentDownload();
+    return _current;
 }
 
 QStringList
 Queue::paths() {
-    Q_D(Queue);
-    return d->paths();
+    return _sortedPaths;
 }
 
-
 QHash<QString, Download*>
 Queue::downloads() {
-    Q_D(Queue);
-    return d->downloads();
+    QHash<QString, Download*> downloads;
+    foreach(const QString& path, _sortedPaths) {
+        downloads[path] = _downloads[path];
+    }
+    return downloads;
 }
 
 int
 Queue::size() {
-    Q_D(Queue);
-    return d->size();
-}
+    return _downloads.size();
+}
+
+void
+Queue::onDownloadStateChanged() {
+    TRACE;
+    // get the appdownload that emited the signal and
+    // decide what to do with it
+    Download* down = qobject_cast<Download*>(sender());
+    switch (down->state()) {
+        case Download::START:
+            // only start the download in the update method
+            if (_current.isEmpty())
+                updateCurrentDownload();
+            break;
+        case Download::PAUSE:
+            down->pauseDownload();
+            if (!_current.isEmpty()  && _current == down->path())
+                updateCurrentDownload();
+            break;
+        case Download::RESUME:
+            // only resume the download in the update method
+            if (_current.isEmpty())
+                updateCurrentDownload();
+            break;
+        case Download::CANCEL:
+            // cancel and remove the download
+            down->cancelDownload();
+            if (!_current.isEmpty() && _current == down->path())
+                updateCurrentDownload();
+            else
+                remove(down->path());
+            break;
+        case Download::ERROR:
+        case Download::FINISH:
+            // remove the registered object in dbus, remove the download
+            // and the adapter from the list
+            if (!_current.isEmpty() && _current == down->path())
+                updateCurrentDownload();
+            break;
+        default:
+            // do nothing
+            break;
+    }
+}
+
+void
+Queue::onCurrentNetworkModeChanged(QNetworkInfo::NetworkMode mode) {
+    TRACE << mode;
+    if (mode != QNetworkInfo::UnknownMode) {
+        updateCurrentDownload();
+    }
+}
+
+void
+Queue::updateCurrentDownload() {
+    TRACE;
+    if (!_current.isEmpty()) {
+        // check if it was canceled/finished
+        Download* currentDownload = _downloads[_current];
+        Download::State state = currentDownload->state();
+        if (state == Download::CANCEL || state == Download::FINISH
+            || state == Download::ERROR) {
+            qDebug() << "State is CANCEL || FINISH || ERROR";
+            remove(_current);
+            _current = "";
+        } else if (!currentDownload->canDownload()
+                || state == Download::PAUSE) {
+            qDebug() << "States is Cannot Download || PAUSE";
+            _current = "";
+        } else {
+            return;
+        }
+    }
+
+    // loop via the downloads and choose the first that is
+    // started or resumed
+    foreach(const QString& path, _sortedPaths) {
+        Download* down = _downloads[path];
+        Download::State state = down->state();
+        if (down->canDownload()
+                && (state == Download::START
+                    || state == Download::RESUME)) {
+            _current = path;
+            if (state == Download::START)
+                down->startDownload();
+            else
+                down->resumeDownload();
+            break;
+        }
+    }
+
+    emit currentChanged(_current);
+}
+
 
 }  // DownloadManager
 
 }  // Ubuntu
-
-#include "moc_queue.cpp"

=== modified file 'libubuntudownloadmanager/downloads/queue.h'
--- libubuntudownloadmanager/downloads/queue.h	2013-11-25 14:46:52 +0000
+++ libubuntudownloadmanager/downloads/queue.h	2013-12-12 16:41:39 +0000
@@ -32,10 +32,8 @@
 
 using namespace System;
 
-class QueuePrivate;
 class Queue : public QObject {
     Q_OBJECT
-    Q_DECLARE_PRIVATE(Queue)
 
  public:
     explicit Queue(SystemNetworkInfo* networkInfo,
@@ -56,12 +54,16 @@
     void downloadRemoved(QString path);
 
  private:
-    Q_PRIVATE_SLOT(d_func(), void onDownloadStateChanged())
-    Q_PRIVATE_SLOT(d_func(),
-            void onCurrentNetworkModeChanged(QNetworkInfo::NetworkMode mode))
+    void onDownloadStateChanged();
+    void onCurrentNetworkModeChanged(QNetworkInfo::NetworkMode mode);
+    void remove(const QString& path);
+    void updateCurrentDownload();
 
  private:
-    QueuePrivate* d_ptr;
+    QString _current;
+    QHash<QString, Download*> _downloads;  // quick for access
+    QStringList _sortedPaths;  // keep the order
+    SystemNetworkInfo* _networkInfo;
 };
 
 }  // DownloadManager