D11389: MPRIS control: do not accumulate interface objects

Friedrich W. H. Kossebau noreply at phabricator.kde.org
Fri Mar 16 18:09:13 UTC 2018


kossebau added a comment.


  Thanks for first review :)

INLINE COMMENTS

> mtijink wrote in mpriscontrolplugin.cpp:118
> This loop can be replaced with `std::find_if` (also elsewhere in the code).

I considered that, but did not find an example with QHash, so was unsure if that works in all versions of Qt that should be supported. Also from what i sketched did it not really simplify the code. But will try, then we/you can compare & comment :)

> mtijink wrote in mpriscontrolplugin.cpp:230
> Instead of using raw `delete`s here, it's better to use `std::unique_ptr` (or a Qt equivalent?).

You are right, it makes sense to move memory management completely over to MprisPlayer instances (shared pointer though, given MprisPlayer is moved around by value). To shy before to go the full path :)

REPOSITORY
  R224 KDE Connect

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

To: kossebau, #kde_connect, mtijink
Cc: mtijink
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180316/627d01af/attachment-0001.html>


More information about the KDEConnect mailing list