D10085: Faster drag&drop in directories with thousands of files

[Elvis Angelaccio]

elvisangelaccio added a comment.


  Almost there, I could not find regressions this time :)
  
  Please fix the coding style and then it's good to go from my side.

INLINE COMMENTS

> draganddrophelper.cpp:36
>  {
> -    return std::find_if(urls.constBegin(), urls.constEnd(), [destUrl](const QUrl& url) {
> -        return url.matches(destUrl, QUrl::StripTrailingSlash);
> -    }) != urls.constEnd();
> +    auto iteratorResult = m_cacheUrlListMatchesUrl.find(destUrl);
> +    // if m_cacheUrlListMatchesUrl does not contains destUrl

Please use `constFind()`

> draganddrophelper.cpp:38-47
> +    if (iteratorResult == m_cacheUrlListMatchesUrl.end()) {
> +        // return the value inserted, that is calculated
> +        return *m_cacheUrlListMatchesUrl.insert(destUrl,
> +            // Iterating the list until a node matches destUrl
> +            std::find_if(urls.constBegin(), urls.constEnd(), [destUrl](const QUrl& url) {
> +                return url.matches(destUrl, QUrl::StripTrailingSlash);
> +            // and comparing it with the imaginary end of the list

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);
  }

> draganddrophelper.cpp:56
>          }
> -
>          // Drop into a directory or a desktop-file

Unrelated whitespace change

> draganddrophelper.h:61
> +     */
> +    static void clearCacheUrlListMatchesUrl();
> +private:

Please call it `clearUrlListMatchesUrlCache()`, to make it slightly more descriptive.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D10085

To: jtamate, #dolphin, elvisangelaccio, markg
Cc: markg, anthonyfieroni, michaelh, elvisangelaccio, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180204/e9092a30/attachment.htm>