D8493: Make Folder View screen aware

Anthony Fieroni noreply at phabricator.kde.org
Fri Nov 3 21:00:04 UTC 2017


anthonyfieroni added inline comments.

INLINE COMMENTS

> foldermodel.cpp:163-164
> +    if (m_screenMapper) {
> +        disconnect(m_screenMapper, &ScreenMapper::screensChanged, this, &FolderModel::invalidateFilter);
> +        disconnect(m_screenMapper, &ScreenMapper::screenMappingChanged, this, &FolderModel::invalidateFilter);
> +        m_screenMapper->removeScreen(m_screen, url());

When QObject dies it's disconnected to all signal/slots. In this case if you want to not notify FolderModel you can use

  m_screenMapper->disconnect(this);

> foldermodel.cpp:1673-1674
> +{
> +    if (m_screenMapper == screenMapper)
> +        return;
> +

{} braces even on one line, done it on all places.

> screenmapper.cpp:32-36
> +    if (s_instance)
> +        return s_instance;
> +
> +    s_instance = new ScreenMapper();
> +    return s_instance;

When you use singleton it's better to make variable construction in one line

  static ScreenMapper *s_instance = new ScreenMapper();
  return s_instance;

or

  static ScreenMapper s_instance;
  return &s_instance;

In this way you don't have unwanted check for creation and variable at class scope.

REPOSITORY
  R119 Plasma Desktop

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

To: amantia, #plasma, ervin, mlaurent, dvratil, hein, aacid, davidedmundson, apol, mwolff
Cc: anthonyfieroni, ngraham, mwolff, davidedmundson, broulik, mart, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171103/4428dd48/attachment.html>


More information about the Plasma-devel mailing list