[Differential] [Changed Subscribers] D1366: Add Event Sounds stream to Applications list

[markg (Mark Gaiser)]

markg added inline comments.

INLINE COMMENTS
  src/context.cpp:424 You can merge this one and the earlier isNew i think.
  
  Something like:
  
  ```
  auto result = m_sinkInputs.data().constFind(eventRoleIndex); 
  
  if (result == m_sinkInputs.data().constEnd()) {
      emit m_sinkInputs.added(... something ...);
  } else {
      emit m_sinkInputs.updated(result.value() .. or something like it);
  }
  ```
  
  You have to change it obviously, but you  can make it nicer by using constFind
  src/kcm/package/contents/ui/StreamListItem.qml:57-65 Just a preference, feel free to ignore.
  Can you try to make get this text value from the C++ side instead of if/elseif/else in QML.. Would be cleaner.
  
  In fact, you might be able to tweak PulseObject.name and and just use that as text.
  src/maps.h:67 This isn't a neat way.
  
  You're returning the data in a writeable way so that you can add items later on. I think it would be cleaner and better if you simply add an "insertEntry" function (just like you already have an updateEntry and a removeEntry).  Then use the new "insertEntry" where you use this data() method instead.
  src/maps.h:75-78 Big pros if you rewrite this to a:
  
  
  ```
  auto result = m_data.constFind(t);
  
  if (result != m_data.constEnd()) {
      return result.value();
  } else {
      return -1;
  }
  ```
  
  Or something alike. My example probably doesn't work _exactly_ like that ;)
  src/maps.h:158 Massive +1 (i know you didn't touch this here.. just commenting on it since i'm reading the code now).
  
  If you transform this into smart pointers you save quite some code within the removeEntry and the reset method. And you prevent accidental "oops, forgot to delete the object" mistakes (aka, memory leaks).
  
  Note: that could also make m_pendingRemovals redundant. But you would have to test that.

REPOSITORY
  rPLASMAPA Plasma Audio Volume Applet

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: drosca, Plasma, Plasma: Design
Cc: markg, broulik, colomar, plasma-devel, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160515/a0b4b16f/attachment.html>