Deal with API changes due to subaddresses, with no visible change #908
Conversation
src/libwalletqt/Subaddress.cpp
Outdated
|
|
||
| Monero::SubaddressRow * Subaddress::getRow(int index) const | ||
| { | ||
| return m_rows.at(index); |
There was a problem hiding this comment.
Does this need a range test ?
There was a problem hiding this comment.
Range check was already done in the caller's side, but added a range test anyway.
src/libwalletqt/Subaddress.h
Outdated
| explicit Subaddress(Monero::Subaddress * subaddressImpl, QObject *parent); | ||
| friend class Wallet; | ||
| Monero::Subaddress * m_subaddressImpl; | ||
| mutable QList<Monero::SubaddressRow*> m_rows; |
There was a problem hiding this comment.
Why is this mutable ? If it's because addRow/setLabel/refresh is const, then why are those const ?
There was a problem hiding this comment.
I removed mutable, and made those member functions non-const.
| { | ||
| m_subaddressAccountImpl->setLabel(accountIndex, label.toStdString()); | ||
| getAll(true); | ||
| } |
There was a problem hiding this comment.
That code looks a lot like code above. Can it be factored ? Especially the getAll functions.
There was a problem hiding this comment.
I don't know how they can be refactored. Personally I also don't find them too redundant.
src/libwalletqt/SubaddressAccount.h
Outdated
| explicit SubaddressAccount(Monero::SubaddressAccount * subaddressAccountImpl, QObject *parent); | ||
| friend class Wallet; | ||
| Monero::SubaddressAccount * m_subaddressAccountImpl; | ||
| mutable QList<Monero::SubaddressAccountRow*> m_rows; |
There was a problem hiding this comment.
same comment about mutable
src/libwalletqt/Wallet.cpp
Outdated
| } | ||
|
|
||
| SubaddressModel *Wallet::subaddressModel() const | ||
| { |
There was a problem hiding this comment.
Given this does not return a const SuaddressModel, and it const casts this below, should this const be removed ?
There was a problem hiding this comment.
I think this member function was made const and the m_subaddressAccountModel member data mutable because m_subaddressAccountModel gets assigned a newly instantiated object when it's pointed to nullptr (i.e., NOT because it returns a non-const SubaddressModel* -- it's perfectly OK for a const member function to return a (copy of ) member data which is a pointer to a non-const object). But anyway I made those functions (subaddress(), subaddressModel(), subaddressAccount(), subaddressAccountModel()) non-const and removed mutable, as I don't see any problem in doing so.
src/libwalletqt/Wallet.h
Outdated
| Subaddress * m_subaddress; | ||
| mutable SubaddressModel * m_subaddressModel; | ||
| SubaddressAccount * m_subaddressAccount; | ||
| mutable SubaddressAccountModel * m_subaddressAccountModel; |
There was a problem hiding this comment.
These mutable qualifiers would like go with the const on the getter above removed
src/model/SubaddressAccountModel.h
Outdated
|
|
||
| public: | ||
| enum SubaddressAccountRowRole { | ||
| SubaddressAccountRole = Qt::UserRole + 1, // for the SubaddressAccountRow object; |
There was a problem hiding this comment.
This one is not handled in roleNames and data. Is it a placeholder only?
|
Looks good to me. Can be merged once @moneromooo-monero's remarks are addressed/fixed. |
|
@kenshi84 This PR becomes obsolete when monero-project/monero#2703 is merged right? |
|
@Jaqueeee Correct, so closing this. Comments from @moneromooo-monero will be reflected in #793 . |
No description provided.