D24213: Add live conversions between numerical bases

[Rolf Eike Beer]

dakon requested changes to this revision.
dakon added a comment.
This revision now requires changes to proceed.


  I personally like it and it would often be helpful for my personal use, so I guess if noone complains just go for it.

INLINE COMMENTS

> kcalc.cpp:1882
> +void KCalculator::slotBaseModeAmountChanged(KNumber number) {
> +	QString converted = QStringLiteral("%1");
> +	QString decimal_str = QString(converted).arg(number.toUint64(), 0, 10);

converted is not needed, just put the QStringLiteral in the next line where it is used.

> kcalc.cpp:1982
>  
> +		foreach(QLabel* lbl, base_conversion_labels_) {
> +			lbl->show();

and this becomes bad if this stays a QList, because it detaches (i.e. copies) the container for the iteration, it needs at least qAsConst(). No problem if it is a std::array as those do not detach, but the Qt types need this.

> kcalc.cpp:2006
>  
> +		foreach(QLabel* lbl, base_conversion_labels_) {
> +		lbl->hide();

same here

> kcalc.h:270
>  
> +    QList<QLabel*> base_conversion_labels_;
> +

even if the code above also uses it, QList is the most awful container one can use. Either use QVector or just use std::array, this has a fixed size and needs no dynamic resizing. A followup patch to convert the QLists above would be welcome ;)

REPOSITORY
  R353 KCalc

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

To: umanovskis, #vdg, cfeck, dakon
Cc: dakon, kde-utils-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20191003/477c9bcd/attachment-0001.html>