Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qt: function filterAcceptsRow seems to be unused #18860

Closed
brakmic opened this issue May 3, 2020 · 9 comments
Closed

qt: function filterAcceptsRow seems to be unused #18860

brakmic opened this issue May 3, 2020 · 9 comments

Comments

@brakmic
Copy link
Contributor

brakmic commented May 3, 2020

I have read in @practicalswift 's Issue #18858 that there was an unused function found, so I thought, I could maybe also run a few simple searches through the code. Just for the sake of learning. For that I used cppcheck.

cppcheck --enable=unusedFunction src/qt

Of course, there are many false positives, but one function seems to be lying dormant for quite a long time: filterAcceptsRow from qt/transactionfilterproxy.cpp.

Also, there is another one in addressbookpage.cpp. However, this one seems to have only a definition. I could not find its corresponding class member declaration. Not sure why, but there isn't one in addressbookpage.h.

Another interesting aspect is, that filterAcceptsRow from transactionfilterproxy.cpp was present since 0.15, without ever having been used, if I am not mistaken. And there were no changes until 0.17, when the other variant from addressbookpage.cpp got implemented.

I don't understand the reasons, and it could be that I am simply missing the forest for the trees, but I could compile, run and also qt-test my modified code without any failure. That is, without having compiled filterAcceptsRow in both of the classes.

However, before opening a PR, I would like to ask experienced people for advice. Is there anything that I might have been missed? Do these functions maybe exist for testing purposes only?

Regards,

---EDIT:

There are a few more of them, but I did not check them thoroughly. Will have to recompile/retest the code first.

src/qt/rpcconsole.cpp:123:0: style: The function 'Name' is never used. [unusedFunction]

^
src/qt/rpcconsole.cpp:124:0: style: The function 'NewTimer' is never used. [unusedFunction]

^
src/qt/bitcoinamountfield.cpp:45:0: style: The function 'fixup' is never used. [unusedFunction]

^
src/qt/bitcoinamountfield.cpp:67:0: style: The function 'stepBy' is never used. [unusedFunction]

^
src/qt/bitcoinamountfield.cpp:167:0: style: The function 'stepEnabled' is never used. [unusedFunction]
@maflcko
Copy link
Member

maflcko commented May 3, 2020

The function is virtual, see https://doc.qt.io/qt-5/qsortfilterproxymodel.html#filterAcceptsRow

@brakmic
Copy link
Contributor Author

brakmic commented May 3, 2020

The function is virtual, see https://doc.qt.io/qt-5/qsortfilterproxymodel.html#filterAcceptsRow

Ok, now I understand.
Before C++11 there was no override keyword so it was a bit harder to distinguish between such functions and non-overridden ones.

@promag
Copy link
Member

promag commented May 3, 2020

TransactionFilterProxy::filterAcceptsRow is called when any model filter is changed (when invalidateFilter is called). For instance see TransactionFilterProxy::setSearchString.

@brakmic
Copy link
Contributor Author

brakmic commented May 3, 2020

TransactionFilterProxy::filterAcceptsRow is called when any model filter is changed (when invalidateFilter is called). For instance see TransactionFilterProxy::setSearchString.

Yes, I see it. In several places. Qt Framework magic.
Thanks for the explanation.
Maybe it should be written down, somewhere, like a Qt/UI-doc.

@practicalswift
Copy link
Contributor

@brakmic If you want to investigate unused functions generally in our code base then you might want to take a look at #18670 (comment) :)

@brakmic
Copy link
Contributor Author

brakmic commented May 3, 2020

@brakmic If you want to investigate unused functions generally in our code base then you might want to take a look at #18670 (comment) :)

Wow, that's an impressive collection you have there. :)
Many thanks!

@promag
Copy link
Member

promag commented May 3, 2020

Having override would be nice. I think this can be closed.

@brakmic brakmic closed this as completed May 3, 2020
@practicalswift
Copy link
Contributor

practicalswift commented May 3, 2020

@brakmic

Wow, that's an impressive collection you have there. :)

Please note that the list in #18670 (comment) is only a subset all unused functions.

Proof: CCoinsViewCache::GetValueIn(…) was missing in the list :)

@hebasto
Copy link
Member

hebasto commented May 3, 2020

@promag

Having override would be nice. I think this can be closed.

Do you mean #16710? 😃

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants
@brakmic @promag @maflcko @practicalswift @hebasto and others