D29502: kwidgetsaddons: Add a named colors support in KColorCombo.
Friedrich W. H. Kossebau
noreply at phabricator.kde.org
Sat May 16 13:42:23 BST 2020
kossebau added a comment.
Isn't the recommendation to rather avoid using things like QPair, and instead used properly defined structs? And ideally non-nested ones, to help with cases of forward declarations?
Even QPair's API dox says so:
"The advent of C++11 automatic variable type deduction (auto) shifts the emphasis from the type name to the name of functions and members. Thus, QPair, like std::pair and std::tuple, is mostly useful in generic (template) code, where defining a dedicated type is not possible."
Code which uses ".first" and ".second" is harder to understand. And any users of the new API who want to pass in named colors but who cannot make use of auto-derived type name or init-list constructors, so have to explicitly write the name would also prefer some named type over QPair<QString, QColor>. So please reconsider using some non-nested struct, perhaps named e.g. "KNamedColor".
The alias "ColorList" might be also confusing, as it misses to point out this is a list of named colors, not just a list of colors (one might naively think of QList<QColor>). "NamedColorList" would be less ambiguous.
And as long as we are Qt5 at least., QVector would also be favourable here over QList given the size of the list item type and given that insertions are not expected to be typically done on this type.
REPOSITORY
R236 KWidgetsAddons
BRANCH
named_color_support
REVISION DETAIL
https://phabricator.kde.org/D29502
To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham, #vdg
Cc: kossebau, cblack, broulik, cfeck, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200516/8aa8e02c/attachment.htm>
More information about the Kde-frameworks-devel
mailing list