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

Reply via email to