Merge #556: refactor: Make BitcoinUnits::Unit a scoped enum

0e5dedb qt/wallettests: sort includes (William Casarin)
0554251 qt: Skip displayUnitChanged signal if unit is not actually changed (Hennadii Stepanov)
ffbc2fe qt, refactor: Remove default cases for scoped enum (Hennadii Stepanov)
152d5ba qt, refactor: Remove BitcoinUnits::valid function (Hennadii Stepanov)
aa23960 qt, refactor: Make BitcoinUnits::Unit a scoped enum (Hennadii Stepanov)
75832fd qt: Use QVariant instead of int for BitcoinUnit in QSettings (Hennadii Stepanov)

Pull request description:

  This is a rebased version of #60

  Since Qt 5.5 there are [means](https://doc.qt.io/qt-5/qobject.html#Q_ENUM) to register an enum type with the meta-object system (such enum still lacks an ability to interact with [QSettings::setValue()](https://doc.qt.io/qt-5/qsettings.html#setValue) and [QSettings::value()](https://doc.qt.io/qt-5/qsettings.html#value) without defined stream operators).

  In order to reduce global namespace polluting and to force strong type checking, this PR makes BitcoinUnits::Unit a scoped enum (typedef BitcoinUnits::Unit BitcoinUnit;).

  No behavior change.

ACKs for top commit:
  jonatack:
    ACK 0e5dedb, review and debug build of each commit after rebase on current master, lightly tested running the GUI, changing units a few times, and verifying persistence after restarting
  promag:
    Code review ACK 0e5dedb

Tree-SHA512: 39ec0d7e4f0b9b25be287888121a8db6b282339674e37ec3a3554da63a9e22d6fe079e8310ca289b2a0356a19b3c7e55afa17d09dd34e0f222177f603bb053a3

src/qt/bitcoin.cpp

@@ -95,6 +95,8 @@ static void RegisterMetaTypes()
  qRegisterMetaType<std::function<void()>>("std::function<void()>");
  qRegisterMetaType<QMessageBox::Icon>("QMessageBox::Icon");
  qRegisterMetaType<interfaces::BlockAndHeaderTipInfo>("interfaces::BlockAndHeaderTipInfo");
+
+ qRegisterMetaTypeStreamOperators<BitcoinUnit>("BitcoinUnit");
 }
 
 static QString GetLangTerritory()

src/qt/bitcoinamountfield.cpp

@@ -14,6 +14,9 @@
 #include <QHBoxLayout>
 #include <QKeyEvent>
 #include <QLineEdit>
+#include <QVariant>
+
+#include <cassert>
 
 /** QSpinBox that uses fixed-point numbers internally and uses our own
  * formatting/parsing functions.
@@ -96,7 +99,7 @@ class AmountSpinBox: public QAbstractSpinBox
  setValue(val);
  }
 
- void setDisplayUnit(int unit)
+ void setDisplayUnit(BitcoinUnit unit)
  {
  bool valid = false;
  CAmount val = value(&valid);
@@ -122,7 +125,7 @@ class AmountSpinBox: public QAbstractSpinBox
 
  const QFontMetrics fm(fontMetrics());
  int h = lineEdit()->minimumSizeHint().height();
- int w = GUIUtil::TextWidth(fm, BitcoinUnits::format(BitcoinUnits::BTC, BitcoinUnits::maxMoney(), false, BitcoinUnits::SeparatorStyle::ALWAYS));
+ int w = GUIUtil::TextWidth(fm, BitcoinUnits::format(BitcoinUnit::BTC, BitcoinUnits::maxMoney(), false, BitcoinUnits::SeparatorStyle::ALWAYS));
  w += 2; // cursor blinking space
 
  QStyleOptionSpinBox opt;
@@ -148,7 +151,7 @@ class AmountSpinBox: public QAbstractSpinBox
  }
 
 private:
- int currentUnit{BitcoinUnits::BTC};
+ BitcoinUnit currentUnit{BitcoinUnit::BTC};
  CAmount singleStep{CAmount(100000)}; // satoshis
  mutable QSize cachedMinimumSizeHint;
  bool m_allow_empty{true};
@@ -326,14 +329,14 @@ void BitcoinAmountField::unitChanged(int idx)
  unit->setToolTip(unit->itemData(idx, Qt::ToolTipRole).toString());
 
  // Determine new unit ID
- int newUnit = unit->itemData(idx, BitcoinUnits::UnitRole).toInt();
-
- amount->setDisplayUnit(newUnit);
+ QVariant new_unit = unit->currentData(BitcoinUnits::UnitRole);
+ assert(new_unit.isValid());
+ amount->setDisplayUnit(new_unit.value<BitcoinUnit>());
 }
 
-void BitcoinAmountField::setDisplayUnit(int newUnit)
+void BitcoinAmountField::setDisplayUnit(BitcoinUnit new_unit)
 {
- unit->setValue(newUnit);
+ unit->setValue(QVariant::fromValue(new_unit));
 }
 
 void BitcoinAmountField::setSingleStep(const CAmount& step)

src/qt/bitcoinamountfield.h

@@ -6,6 +6,7 @@
 #define BITCOIN_QT_BITCOINAMOUNTFIELD_H
 
 #include <consensus/amount.h>
+#include <qt/bitcoinunits.h>
 
 #include <QWidget>
 
@@ -52,7 +53,7 @@ class BitcoinAmountField: public QWidget
  bool validate();
 
  /** Change unit used to display amount. */
- void setDisplayUnit(int unit);
+ void setDisplayUnit(BitcoinUnit new_unit);
 
  /** Make field empty and ready for new input. */
  void clear();

src/qt/bitcoingui.cpp

@@ -1245,7 +1245,7 @@ void BitcoinGUI::showEvent(QShowEvent *event)
 }
 
 #ifdef ENABLE_WALLET
-void BitcoinGUI::incomingTransaction(const QString& date, int unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName)
+void BitcoinGUI::incomingTransaction(const QString& date, BitcoinUnit unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName)
 {
  // On new transaction, make an info balloon
  QString msg = tr("Date: %1\n").arg(date) +
@@ -1496,11 +1496,10 @@ UnitDisplayStatusBarControl::UnitDisplayStatusBarControl(const PlatformStyle *pl
 {
  createContextMenu();
  setToolTip(tr("Unit to show amounts in. Click to select another unit."));
- QList<BitcoinUnits::Unit> units = BitcoinUnits::availableUnits();
+ QList<BitcoinUnit> units = BitcoinUnits::availableUnits();
  int max_width = 0;
  const QFontMetrics fm(font());
- for (const BitcoinUnits::Unit unit : units)
- {
+ for (const BitcoinUnit unit : units) {
  max_width = qMax(max_width, GUIUtil::TextWidth(fm, BitcoinUnits::longName(unit)));
  }
  setMinimumSize(max_width, 0);
@@ -1530,8 +1529,8 @@ void UnitDisplayStatusBarControl::changeEvent(QEvent* e)
 void UnitDisplayStatusBarControl::createContextMenu()
 {
  menu = new QMenu(this);
- for (const BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) {
- menu->addAction(BitcoinUnits::longName(u))->setData(QVariant(u));
+ for (const BitcoinUnit u : BitcoinUnits::availableUnits()) {
+ menu->addAction(BitcoinUnits::longName(u))->setData(QVariant::fromValue(u));
  }
  connect(menu, &QMenu::triggered, this, &UnitDisplayStatusBarControl::onMenuSelection);
 }
@@ -1552,7 +1551,7 @@ void UnitDisplayStatusBarControl::setOptionsModel(OptionsModel *_optionsModel)
 }
 
 /** When Display Units are changed on OptionsModel it will refresh the display text of the control on the status bar */
-void UnitDisplayStatusBarControl::updateDisplayUnit(int newUnits)
+void UnitDisplayStatusBarControl::updateDisplayUnit(BitcoinUnit newUnits)
 {
  setText(BitcoinUnits::longName(newUnits));
 }

src/qt/bitcoingui.h

@@ -9,6 +9,7 @@
 #include <config/bitcoin-config.h>
 #endif
 
+#include <qt/bitcoinunits.h>
 #include <qt/guiutil.h>
 #include <qt/optionsdialog.h>
 
@@ -260,7 +261,7 @@ public Q_SLOTS:
  bool handlePaymentRequest(const SendCoinsRecipient& recipient);
 
  /** Show incoming transaction notification for new transactions. */
- void incomingTransaction(const QString& date, int unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName);
+ void incomingTransaction(const QString& date, BitcoinUnit unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName);
 #endif // ENABLE_WALLET
 
 private:
@@ -341,7 +342,7 @@ class UnitDisplayStatusBarControl : public QLabel
 
 private Q_SLOTS:
  /** When Display Units are changed on OptionsModel it will refresh the display text of the control on the status bar */
- void updateDisplayUnit(int newUnits);
+ void updateDisplayUnit(BitcoinUnit newUnits);
  /** Tells underlying optionsModel to update its current display unit. */
  void onMenuSelection(QAction* action);
 };

src/qt/bitcoinunits.cpp

@@ -18,94 +18,75 @@ BitcoinUnits::BitcoinUnits(QObject *parent):
 {
 }
 
-QList<BitcoinUnits::Unit> BitcoinUnits::availableUnits()
+QList<BitcoinUnit> BitcoinUnits::availableUnits()
 {
- QList<BitcoinUnits::Unit> unitlist;
- unitlist.append(BTC);
- unitlist.append(mBTC);
- unitlist.append(uBTC);
- unitlist.append(SAT);
+ QList<BitcoinUnit> unitlist;
+ unitlist.append(Unit::BTC);
+ unitlist.append(Unit::mBTC);
+ unitlist.append(Unit::uBTC);
+ unitlist.append(Unit::SAT);
  return unitlist;
 }
 
-bool BitcoinUnits::valid(int unit)
+QString BitcoinUnits::longName(Unit unit)
 {
- switch(unit)
- {
- case BTC:
- case mBTC:
- case uBTC:
- case SAT:
- return true;
- default:
- return false;
- }
-}
-
-QString BitcoinUnits::longName(int unit)
-{
- switch(unit)
- {
- case BTC: return QString("BTC");
- case mBTC: return QString("mBTC");
- case uBTC: return QString::fromUtf8("µBTC (bits)");
- case SAT: return QString("Satoshi (sat)");
- default: return QString("???");
- }
+ switch (unit) {
+ case Unit::BTC: return QString("BTC");
+ case Unit::mBTC: return QString("mBTC");
+ case Unit::uBTC: return QString::fromUtf8("µBTC (bits)");
+ case Unit::SAT: return QString("Satoshi (sat)");
+ } // no default case, so the compiler can warn about missing cases
+ assert(false);
 }
 
-QString BitcoinUnits::shortName(int unit)
+QString BitcoinUnits::shortName(Unit unit)
 {
- switch(unit)
- {
- case uBTC: return QString::fromUtf8("bits");
- case SAT: return QString("sat");
- default: return longName(unit);
- }
+ switch (unit) {
+ case Unit::BTC: return longName(unit);
+ case Unit::mBTC: return longName(unit);
+ case Unit::uBTC: return QString("bits");
+ case Unit::SAT: return QString("sat");
+ } // no default case, so the compiler can warn about missing cases
+ assert(false);
 }
 
-QString BitcoinUnits::description(int unit)
+QString BitcoinUnits::description(Unit unit)
 {
- switch(unit)
- {
- case BTC: return QString("Bitcoins");
- case mBTC: return QString("Milli-Bitcoins (1 / 1" THIN_SP_UTF8 "000)");
- case uBTC: return QString("Micro-Bitcoins (bits) (1 / 1" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)");
- case SAT: return QString("Satoshi (sat) (1 / 100" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)");
- default: return QString("???");
- }
+ switch (unit) {
+ case Unit::BTC: return QString("Bitcoins");
+ case Unit::mBTC: return QString("Milli-Bitcoins (1 / 1" THIN_SP_UTF8 "000)");
+ case Unit::uBTC: return QString("Micro-Bitcoins (bits) (1 / 1" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)");
+ case Unit::SAT: return QString("Satoshi (sat) (1 / 100" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)");
+ } // no default case, so the compiler can warn about missing cases
+ assert(false);
 }
 
-qint64 BitcoinUnits::factor(int unit)
+qint64 BitcoinUnits::factor(Unit unit)
 {
- switch(unit)
- {
- case BTC: return 100000000;
- case mBTC: return 100000;
- case uBTC: return 100;
- case SAT: return 1;
- default: return 100000000;
- }
+ switch (unit) {
+ case Unit::BTC: return 100'000'000;
+ case Unit::mBTC: return 100'000;
+ case Unit::uBTC: return 100;
+ case Unit::SAT: return 1;
+ } // no default case, so the compiler can warn about missing cases
+ assert(false);
 }
 
-int BitcoinUnits::decimals(int unit)
+int BitcoinUnits::decimals(Unit unit)
 {
- switch(unit)
- {
- case BTC: return 8;
- case mBTC: return 5;
- case uBTC: return 2;
- case SAT: return 0;
- default: return 0;
- }
+ switch (unit) {
+ case Unit::BTC: return 8;
+ case Unit::mBTC: return 5;
+ case Unit::uBTC: return 2;
+ case Unit::SAT: return 0;
+ } // no default case, so the compiler can warn about missing cases
+ assert(false);
 }
 
-QString BitcoinUnits::format(int unit, const CAmount& nIn, bool fPlus, SeparatorStyle separators, bool justify)
+QString BitcoinUnits::format(Unit unit, const CAmount& nIn, bool fPlus, SeparatorStyle separators, bool justify)
 {
  // Note: not using straight sprintf here because we do NOT want
  // localized number formatting.
- if(!valid(unit))
- return QString(); // Refuse to format invalid unit
  qint64 n = (qint64)nIn;
  qint64 coin = factor(unit);
  int num_decimals = decimals(unit);
@@ -147,19 +128,19 @@ QString BitcoinUnits::format(int unit, const CAmount& nIn, bool fPlus, Separator
 // Please take care to use formatHtmlWithUnit instead, when
 // appropriate.
 
-QString BitcoinUnits::formatWithUnit(int unit, const CAmount& amount, bool plussign, SeparatorStyle separators)
+QString BitcoinUnits::formatWithUnit(Unit unit, const CAmount& amount, bool plussign, SeparatorStyle separators)
 {
  return format(unit, amount, plussign, separators) + QString(" ") + shortName(unit);
 }
 
-QString BitcoinUnits::formatHtmlWithUnit(int unit, const CAmount& amount, bool plussign, SeparatorStyle separators)
+QString BitcoinUnits::formatHtmlWithUnit(Unit unit, const CAmount& amount, bool plussign, SeparatorStyle separators)
 {
  QString str(formatWithUnit(unit, amount, plussign, separators));
  str.replace(QChar(THIN_SP_CP), QString(THIN_SP_HTML));
  return QString("<span style='white-space: nowrap;'>%1</span>").arg(str);
 }
 
-QString BitcoinUnits::formatWithPrivacy(int unit, const CAmount& amount, SeparatorStyle separators, bool privacy)
+QString BitcoinUnits::formatWithPrivacy(Unit unit, const CAmount& amount, SeparatorStyle separators, bool privacy)
 {
  assert(amount >= 0);
  QString value;
@@ -171,10 +152,11 @@ QString BitcoinUnits::formatWithPrivacy(int unit, const CAmount& amount, Separat
  return value + QString(" ") + shortName(unit);
 }
 
-bool BitcoinUnits::parse(int unit, const QString &value, CAmount *val_out)
+bool BitcoinUnits::parse(Unit unit, const QString& value, CAmount* val_out)
 {
- if(!valid(unit) || value.isEmpty())
+ if (value.isEmpty()) {
  return false; // Refuse to parse invalid unit or empty string
+ }
  int num_decimals = decimals(unit);
 
  // Ignore spaces and thin spaces when parsing
@@ -210,14 +192,9 @@ bool BitcoinUnits::parse(int unit, const QString &value, CAmount *val_out)
  return ok;
 }
 
-QString BitcoinUnits::getAmountColumnTitle(int unit)
+QString BitcoinUnits::getAmountColumnTitle(Unit unit)
 {
- QString amountTitle = QObject::tr("Amount");
- if (BitcoinUnits::valid(unit))
- {
- amountTitle += " ("+BitcoinUnits::shortName(unit) + ")";
- }
- return amountTitle;
+ return QObject::tr("Amount") + " (" + shortName(unit) + ")";
 }
 
 int BitcoinUnits::rowCount(const QModelIndex &parent) const
@@ -240,7 +217,7 @@ QVariant BitcoinUnits::data(const QModelIndex &index, int role) const
  case Qt::ToolTipRole:
  return QVariant(description(unit));
  case UnitRole:
- return QVariant(static_cast<int>(unit));
+ return QVariant::fromValue(unit);
  }
  }
  return QVariant();
@@ -250,3 +227,40 @@ CAmount BitcoinUnits::maxMoney()
 {
  return MAX_MONEY;
 }
+
+namespace {
+qint8 ToQint8(BitcoinUnit unit)
+{
+ switch (unit) {
+ case BitcoinUnit::BTC: return 0;
+ case BitcoinUnit::mBTC: return 1;
+ case BitcoinUnit::uBTC: return 2;
+ case BitcoinUnit::SAT: return 3;
+ } // no default case, so the compiler can warn about missing cases
+ assert(false);
+}
+
+BitcoinUnit FromQint8(qint8 num)
+{
+ switch (num) {
+ case 0: return BitcoinUnit::BTC;
+ case 1: return BitcoinUnit::mBTC;
+ case 2: return BitcoinUnit::uBTC;
+ case 3: return BitcoinUnit::SAT;
+ }
+ assert(false);
+}
+} // namespace
+
+QDataStream& operator<<(QDataStream& out, const BitcoinUnit& unit)
+{
+ return out << ToQint8(unit);
+}
+
+QDataStream& operator>>(QDataStream& in, BitcoinUnit& unit)
+{
+ qint8 input;
+ in >> input;
+ unit = FromQint8(input);
+ return in;
+}