D23918: RFC: Drop Outputs class

Vlad Zahorodnii noreply at phabricator.kde.org
Sat Sep 14 11:18:58 BST 2019


zzag added a comment.


  > You need to provide sources for such generic statements.
  
  Sorry to disappoint you but that's an unwritten rule in C++ community. The main reason why one should avoid inheriting qvector or std::vector has something to do with destructors. More specifically, neither one of those container types has virtual destructor. However, you could fix that with private inheritance, which will look very ugly! From design perspective if you want to transform a container you have to use an algorithm rather than inherit from the container type and add a method, i.e. don't go against STL design. It somewhat relates to composition over inheritance [1].
  
  Let's talk about Outputs class specifically. The main problem with it is that it adds only a constructor and nothing more. In the end we have a class without anything new to offer than its base class [QVector<AbstractOutput *>]. The worst thing is that we have yet another container type to deal with. The correct solution is to use a helper function to perform conversions between QVector<Base> and QVector<Derived> rather than using new container type.
  
  [1] https://en.wikipedia.org/wiki/Composition_over_inheritance

INLINE COMMENTS

> romangg wrote in utils.h:240
> For simple data types according to Qt docs resize is faster. It's a micro-optimization, but since the outputs are queried often maybe it's worth it. I assume we could use template specialization for `AbstractOutput*` as `To` type, or just use no template at all? Although having a templated qvector_cast is nice.

We could use std::enable_if or std::enable_if_t to specialize pointer types, however I don't think this level of complexity worth the trouble. We want to copy primitive and complex types.

Append method costs only an if statement and an integer increment operation. We can stick with the current solution unless benchmarks show that we need to make concrete assumptions about internal implementation of qvector.

REPOSITORY
  R108 KWin

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

To: zzag, #kwin
Cc: romangg, davidedmundson, alexeymin, kwin, LeGast00n, The-Feren-OS-Dev, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, crozbo, bwowk, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20190914/5108f06a/attachment.html>


More information about the kwin mailing list