SubaddressAccount: cleanup

src/dialog/AccountSwitcherDialog.cpp

@@ -67,21 +67,25 @@ void AccountSwitcherDialog::switchAccount(const QModelIndex &index) {
 }
 
 void AccountSwitcherDialog::copyLabel() {
-    auto row = this->currentEntry();
-    if (!row) {
+    QModelIndex index = m_proxyModel->mapToSource(ui->accounts->currentIndex());
+    if (!index.isValid()) {
         return;
     }
 
-    Utils::copyToClipboard(row->getLabel());
+    auto& row = m_wallet->subaddressAccountModel()->entryFromIndex(index);
+
+    Utils::copyToClipboard(row.label);
 }
 
 void AccountSwitcherDialog::copyBalance() {
-    auto row = this->currentEntry();
-    if (!row) {
+    QModelIndex index = m_proxyModel->mapToSource(ui->accounts->currentIndex());
+    if (!index.isValid()) {
         return;
     }
 
-    Utils::copyToClipboard(row->getBalance());
+    auto& row = m_wallet->subaddressAccountModel()->entryFromIndex(index);
+
+    Utils::copyToClipboard(WalletManager::displayAmount(row.balance));
 }
 
 void AccountSwitcherDialog::editLabel() {
@@ -115,9 +119,4 @@ void AccountSwitcherDialog::showContextMenu(const QPoint &point) {
     menu->popup(ui->accounts->viewport()->mapToGlobal(point));
 }
 
-AccountRow* AccountSwitcherDialog::currentEntry() {
-    QModelIndex index = m_proxyModel->mapToSource(ui->accounts->currentIndex());
-    return m_wallet->subaddressAccountModel()->entryFromIndex(index);
-}
-
 AccountSwitcherDialog::~AccountSwitcherDialog() = default;

src/dialog/AccountSwitcherDialog.h

@@ -35,8 +35,6 @@ private:
     void copyBalance();
     void editLabel();
 
-    AccountRow* currentEntry();
-
     QScopedPointer<Ui::AccountSwitcherDialog> ui;
     Wallet *m_wallet;
     SubaddressAccountModel *m_model;

src/libwalletqt/SubaddressAccount.cpp

@@ -4,24 +4,12 @@
 #include "SubaddressAccount.h"
 #include <wallet/wallet2.h>
 
-SubaddressAccount::SubaddressAccount(Wallet *wallet, tools::wallet2 *wallet2, QObject *parent)
+SubaddressAccount::SubaddressAccount(tools::wallet2 *wallet2, QObject *parent)
     : QObject(parent)
-    , m_wallet(wallet)
     , m_wallet2(wallet2)
 {
 }
 
-bool SubaddressAccount::getRow(int index, std::function<void (AccountRow &row)> callback) const
-{
-    if (index < 0 || index >= m_rows.size())
-    {
-        return false;
-    }
-
-    callback(*m_rows.value(index));
-    return true;
-}
-
 void SubaddressAccount::addRow(const QString &label)
 {
     m_wallet2->add_subaddress_account(label.toStdString());
@@ -39,16 +27,14 @@ void SubaddressAccount::refresh()
     emit refreshStarted();
 
     this->clearRows();
+
     for (uint32_t i = 0; i < m_wallet2->get_num_subaddress_accounts(); ++i)
     {
-        auto *row = new AccountRow{this,
-                                   i,
-                                   QString::fromStdString(m_wallet2->get_subaddress_as_str({i,0})),
-                                   QString::fromStdString(m_wallet2->get_subaddress_label({i,0})),
-                                   m_wallet2->balance(i, false),
-                                   m_wallet2->unlocked_balance(i, false)};
-
-        m_rows.append(row);
+        m_rows.emplace_back(
+            QString::fromStdString(m_wallet2->get_subaddress_as_str({i,0})),
+            QString::fromStdString(m_wallet2->get_subaddress_label({i,0})),
+            m_wallet2->balance(i, false),
+            m_wallet2->unlocked_balance(i, false));
     }
 
     emit refreshFinished();
@@ -61,11 +47,18 @@ qsizetype SubaddressAccount::count() const
 
 void SubaddressAccount::clearRows()
 {
-    qDeleteAll(m_rows);
     m_rows.clear();
 }
 
-AccountRow* SubaddressAccount::row(int index) const
+const QList<AccountRow>& SubaddressAccount::getRows()
 {
-    return m_rows.value(index);
-}
+    return m_rows;
+}
+
+const AccountRow& SubaddressAccount::row(const int index) const
+{
+    if (index < 0 || index >= m_rows.size()) {
+        throw std::out_of_range("Index out of range");
+    }
+    return m_rows[index];
+}

src/libwalletqt/SubaddressAccount.h

@@ -4,14 +4,9 @@
 #ifndef SUBADDRESSACCOUNT_H
 #define SUBADDRESSACCOUNT_H
 
-#include <functional>
-
 #include <QObject>
-#include <QReadWriteLock>
 #include <QList>
-#include <QDateTime>
 
-#include "Wallet.h"
 #include "rows/AccountRow.h"
 
 namespace tools {
@@ -23,30 +18,26 @@ class SubaddressAccount : public QObject
     Q_OBJECT
 
 public:
-    void getAll() const;
-    bool getRow(int index, std::function<void (AccountRow &row)> callback) const;
-    void addRow(const QString &label);
+    const QList<AccountRow>& getRows();
+    const AccountRow& row(int index) const;
 
+    void addRow(const QString &label);
     void setLabel(quint32 accountIndex, const QString &label);
+    qsizetype count() const;
 
     void refresh();
-
-    qsizetype count() const;
     void clearRows();
 
-    AccountRow* row(int index) const;
-
 signals:
     void refreshStarted() const;
     void refreshFinished() const;
 
 private:
-    explicit SubaddressAccount(Wallet *wallet, tools::wallet2 *wallet2, QObject *parent);
+    explicit SubaddressAccount(tools::wallet2 *wallet2, QObject *parent);
     friend class Wallet;
 
-    Wallet *m_wallet;
     tools::wallet2 *m_wallet2;
-    QList<AccountRow*> m_rows;
+    QList<AccountRow> m_rows;
 };
 
 #endif // SUBADDRESSACCOUNT_H

src/libwalletqt/Wallet.cpp

@@ -45,7 +45,7 @@ Wallet::Wallet(Monero::Wallet *wallet, QObject *parent)
         , m_connectionStatus(Wallet::ConnectionStatus_Disconnected)
         , m_currentSubaddressAccount(0)
         , m_subaddress(new Subaddress(this, wallet->getWallet(), this))
-        , m_subaddressAccount(new SubaddressAccount(this, wallet->getWallet(), this))
+        , m_subaddressAccount(new SubaddressAccount(wallet->getWallet(), this))
         , m_refreshNow(false)
         , m_refreshEnabled(false)
         , m_scheduler(this)

src/libwalletqt/rows/AccountRow.cpp

@@ -1,25 +0,0 @@
-// SPDX-License-Identifier: BSD-3-Clause
-// SPDX-FileCopyrightText: The Monero Project
-
-#include "AccountRow.h"
-#include "WalletManager.h"
-
-qsizetype AccountRow::getRow() const {
-    return m_row;
-}
-
-const QString& AccountRow::getAddress() const {
-    return m_address;
-}
-
-const QString& AccountRow::getLabel() const {
-    return m_label;
-}
-
-QString AccountRow::getBalance() const {
-    return WalletManager::displayAmount(m_balance);
-}
-
-QString AccountRow::getUnlockedBalance() const {
-    return WalletManager::displayAmount(m_unlockedBalance);
-}

src/libwalletqt/rows/AccountRow.h

@@ -4,33 +4,20 @@
 #ifndef FEATHER_ACCOUNTROW_H
 #define FEATHER_ACCOUNTROW_H
 
-#include <QObject>
+#include <QString>
 
-class AccountRow : public QObject
+struct AccountRow
 {
-Q_OBJECT
+    QString address;
+    QString label;
+    quint64 balance;
+    quint64 unlockedBalance;
 
-public:
-    AccountRow(QObject *parent, qsizetype row, const QString& address, const QString &label, uint64_t balance, uint64_t unlockedBalance)
-            : QObject(parent)
-            , m_row(row)
-            , m_address(address)
-            , m_label(label)
-            , m_balance(balance)
-            , m_unlockedBalance(unlockedBalance) {}
-
-    qsizetype getRow() const;
-    const QString& getAddress() const;
-    const QString& getLabel() const;
-    QString getBalance() const;
-    QString getUnlockedBalance() const;
-
-private:
-    qsizetype m_row;
-    QString m_address;
-    QString m_label;
-    uint64_t m_balance;
-    uint64_t m_unlockedBalance;
+    AccountRow(const QString& address_, const QString &label_, uint64_t balance_, uint64_t unlockedBalance_)
+            : address(address_)
+            , label(label_)
+            , balance(balance_)
+            , unlockedBalance(unlockedBalance_) {}
 };
 
 #endif //FEATHER_ACCOUNTROW_H

src/model/SubaddressAccountModel.cpp

@@ -45,32 +45,28 @@ int SubaddressAccountModel::columnCount(const QModelIndex &parent) const
 
 QVariant SubaddressAccountModel::data(const QModelIndex &index, int role) const
 {
-    if (!index.isValid() || index.row() < 0 || static_cast<quint64>(index.row()) >= m_subaddressAccount->count())
+    const QList<AccountRow>& rows = m_subaddressAccount->getRows();
+    if (index.row() < 0 || index.row() >= rows.size()) {
         return {};
-
-    QVariant result;
-
-    bool found = m_subaddressAccount->getRow(index.row(), [this, &index, &result, &role](const AccountRow &row) {
-        if (role == Qt::DisplayRole || role == Qt::EditRole || role == Qt::UserRole) {
-            result = parseSubaddressAccountRow(row, index, role);
-        }
-        else if (role == Qt::FontRole) {
-            if (index.column() == Column::Balance || index.column() == Column::UnlockedBalance) {
-                result = Utils::getMonospaceFont();
-            }
-        }
-        else if (role == Qt::TextAlignmentRole) {
-            if (index.column() == Column::Balance || index.column() == Column::UnlockedBalance) {
-                result = Qt::AlignRight;
-            }
-        }
-    });
-
-    if (!found) {
-        qCritical("%s: internal error: invalid index %d", __FUNCTION__, index.row());
     }
 
-    return result;
+    const AccountRow& row = rows[index.row()];
+
+    if (role == Qt::DisplayRole || role == Qt::EditRole || role == Qt::UserRole) {
+        return parseSubaddressAccountRow(row, index, role);
+    }
+    else if (role == Qt::FontRole) {
+        if (index.column() == Column::Balance || index.column() == Column::UnlockedBalance) {
+            return Utils::getMonospaceFont();
+        }
+    }
+    else if (role == Qt::TextAlignmentRole) {
+        if (index.column() == Column::Balance || index.column() == Column::UnlockedBalance) {
+            return Qt::AlignRight;
+        }
+    }
+
+    return {};
 }
 
 QVariant SubaddressAccountModel::parseSubaddressAccountRow(const AccountRow &row,
@@ -83,19 +79,19 @@ QVariant SubaddressAccountModel::parseSubaddressAccountRow(const AccountRow &row
             }
             return QString("#%1").arg(QString::number(index.row()));
         case Address:
-            return row.getAddress();
+            return row.address;
         case Label:
-            return row.getLabel();
+            return row.label;
         case Balance:
             if (role == Qt::UserRole) {
-                return WalletManager::amountFromString(row.getBalance());
+                return row.balance;
             }
-            return row.getBalance();
+            return WalletManager::displayAmount(row.balance);
         case UnlockedBalance:
             if (role == Qt::UserRole) {
-                return WalletManager::amountFromString(row.getUnlockedBalance());
+                return row.unlockedBalance;
             }
-            return row.getUnlockedBalance();
+            return WalletManager::displayAmount(row.unlockedBalance);
         default:
             return QVariant();
     }
@@ -143,7 +139,6 @@ bool SubaddressAccountModel::setData(const QModelIndex &index, const QVariant &v
     return false;
 }
 
-
 Qt::ItemFlags SubaddressAccountModel::flags(const QModelIndex &index) const
 {
     if (!index.isValid())
@@ -155,7 +150,7 @@ Qt::ItemFlags SubaddressAccountModel::flags(const QModelIndex &index) const
     return QAbstractTableModel::flags(index);
 }
 
-AccountRow* SubaddressAccountModel::entryFromIndex(const QModelIndex &index) const {
+const AccountRow& SubaddressAccountModel::entryFromIndex(const QModelIndex &index) const {
     return m_subaddressAccount->row(index.row());
 }
 
@@ -163,4 +158,4 @@ SubaddressAccountProxyModel::SubaddressAccountProxyModel(QObject *parent)
     : QSortFilterProxyModel(parent)
 {
     setSortRole(Qt::UserRole);
-}
+}

src/model/SubaddressAccountModel.h

@@ -36,7 +36,7 @@ public:
 
     bool setData(const QModelIndex &index, const QVariant &value, int role) override;
 
-    AccountRow* entryFromIndex(const QModelIndex &index) const;
+    const AccountRow& entryFromIndex(const QModelIndex &index) const;
 
 public slots:
     void startReset();