Differential D10085 Diff 26473 src/views/draganddrophelper.cpp

Changeset View

Standalone View

View Options

src/views/draganddrophelper.cpp

Show All 23 Lines
24#include <QDBusMessage> 24​#include <QDBusMessage>
25#include <QDBusConnection> 25​#include <QDBusConnection>
26#include <QDropEvent> 26​#include <QDropEvent>
27#include <QMimeData> 27​#include <QMimeData>
28 28​
29#include <KIO/DropJob> 29​#include <KIO/DropJob>
30#include <KJobWidgets> 30​#include <KJobWidgets>
31 31​
32​QHash<QUrl, bool> DragAndDropHelper::m_cacheUrlListMatchesUrl;
32 33​
33bool DragAndDropHelper::urlListMatchesUrl(const QList<QUrl>& urls, const QUrl& destUrl) 34​bool DragAndDropHelper::urlListMatchesUrl(const QList<QUrl>& urls, const QUrl& destUrl)
34{ 35​{
35 return std::find_if(urls.constBegin(), urls.constEnd(), [destUrl](const QUrl& url) { 36​ if (!m_cacheUrlListMatchesUrl.contains(destUrl))
37​ {
38​ m_cacheUrlListMatchesUrl.insert(destUrl,
39​ std::find_if(urls.constBegin(), urls.constEnd(), [destUrl](const QUrl& url) {
36 return url.matches(destUrl, QUrl::StripTrailingSlash); 40​ return url.matches(destUrl, QUrl::StripTrailingSlash);
37 }) != urls.constEnd(); 41​ }) != urls.constEnd() );
42​ }
43​ return m_cacheUrlListMatchesUrl[destUrl];
markgUnsubmitted
Done

You now have a double lookup in the m_cacheUrlListMatchesUrl hash. It's fast, so not that big of a deal.
But since we're in the optimizing mood here.. :)

auto result = m_cacheUrlListMatchesUrl.find(destUrl);
if (result != m_cacheUrlListMatchesUrl.end()) {
  return *m_cacheUrlListMatchesUrl.insert(destUrl,
    std::find_if(urls.constBegin(), urls.constEnd(), [destUrl](const QUrl& url) {
      return url.matches(destUrl, QUrl::StripTrailingSlash);
    }) != urls.constEnd() );
} else {
  return *result;
}

The if statement is _really_ ugly now :P
Something along those lines should work.
Note that you can directly return from the insert, it gives an iterator back and that is - if i'm right - the iterator of the just inserted data.

markg: You now have a double lookup in the m_cacheUrlListMatchesUrl hash. It's fast, so not that big…
elvisangelaccioUnsubmitted
Done

Please use constFind()

elvisangelaccio: Please use `constFind()`
38} 44​}
anthonyfieroniUnsubmitted
Done

Don't do that. It's ridiculous, it cannot guarantied that function will be called with same objects. It's enough speed-up with concurrent calls.

anthonyfieroni: Don't do that. It's ridiculous, it cannot guarantied that function will be called with same…
39 45​
40KIO::DropJob* DragAndDropHelper::dropUrls(const QUrl& destUrl, QDropEvent* event, QWidget* window) 46​KIO::DropJob* DragAndDropHelper::dropUrls(const QUrl& destUrl, QDropEvent* event, QWidget* window)
41{ 47​{
elvisangelaccioUnsubmitted
Done

This is really hard to read, and the comments only add more noise. How about doing it like this:

bool DragAndDropHelper::urlListMatchesUrl(const QList<QUrl>& urls, const QUrl& destUrl)
{
    auto iteratorResult = m_cacheUrlListMatchesUrl.constFind(destUrl);
    if (iteratorResult != m_cacheUrlListMatchesUrl.constEnd()) {
        return *iteratorResult;
    }

    const bool destUrlMatches = (std::find_if(urls.constBegin(), urls.constEnd(), [destUrl](const QUrl& url) {
        return url.matches(destUrl, QUrl::StripTrailingSlash);
    }) != urls.constEnd());

    return *m_cacheUrlListMatchesUrl.insert(destUrl, destUrlMatches);
}
elvisangelaccio: This is really hard to read, and the comments only add more noise. How about doing it like this…
42 const QMimeData* mimeData = event->mimeData(); 48​ const QMimeData* mimeData = event->mimeData();
43 if (mimeData->hasFormat(QStringLiteral("application/x-kde-ark-dndextract-service")) && 49​ if (mimeData->hasFormat(QStringLiteral("application/x-kde-ark-dndextract-service")) &&
44 mimeData->hasFormat(QStringLiteral("application/x-kde-ark-dndextract-path"))) { 50​ mimeData->hasFormat(QStringLiteral("application/x-kde-ark-dndextract-path"))) {
45 const QString remoteDBusClient = mimeData->data(QStringLiteral("application/x-kde-ark-dndextract-service")); 51​ const QString remoteDBusClient = mimeData->data(QStringLiteral("application/x-kde-ark-dndextract-service"));
46 const QString remoteDBusPath = mimeData->data(QStringLiteral("application/x-kde-ark-dndextract-path")); 52​ const QString remoteDBusPath = mimeData->data(QStringLiteral("application/x-kde-ark-dndextract-path"));
47 53​
48 QDBusMessage message = QDBusMessage::createMethodCall(remoteDBusClient, remoteDBusPath, 54​ QDBusMessage message = QDBusMessage::createMethodCall(remoteDBusClient, remoteDBusPath,
49 QStringLiteral("org.kde.ark.DndExtract"), QStringLiteral("extractSelectedFilesTo")); 55​ QStringLiteral("org.kde.ark.DndExtract"), QStringLiteral("extractSelectedFilesTo"));
50 message.setArguments({destUrl.toDisplayString(QUrl::PreferLocalFile)}); 56​ message.setArguments({destUrl.toDisplayString(QUrl::PreferLocalFile)});
51 QDBusConnection::sessionBus().call(message); 57​ QDBusConnection::sessionBus().call(message);
52 } else { 58​ } else {
53 if (urlListMatchesUrl(event->mimeData()->urls(), destUrl)) { 59​ if (urlListMatchesUrl(event->mimeData()->urls(), destUrl)) {
54 return nullptr; 60​ return nullptr;
55 } 61​ }
56 ​
elvisangelaccioUnsubmitted
Done

Unrelated whitespace change

elvisangelaccio: Unrelated whitespace change
57 // Drop into a directory or a desktop-file 62​ // Drop into a directory or a desktop-file
58 KIO::DropJob *job = KIO::drop(event, destUrl); 63​ KIO::DropJob *job = KIO::drop(event, destUrl);
59 KJobWidgets::setWindow(job, window); 64​ KJobWidgets::setWindow(job, window);
60 return job; 65​ return job;
61 } 66​ }
62 67​
63 return nullptr; 68​ return nullptr;
64} 69​}
65 70​
71​void DragAndDropHelper::clearCacheUrlListMatchesUrl()
72​{
73​ DragAndDropHelper::m_cacheUrlListMatchesUrl.clear();
74​}
75​