-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
replace QHashIterator with QMultiHash::cbegin/cend #4377
Conversation
QHashIterator<SoundDeviceId, AudioOutput> it(outputs); | ||
while (it.hasNext()) { | ||
it.next(); | ||
auto hash = config.getOutputs(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hash
is not an appropriate variable name, how aboutoutputDeviceMap
?const auto
QHashIterator<SoundDeviceId, AudioInput> it(inputs); | ||
while (it.hasNext()) { | ||
it.next(); | ||
auto hash = config.getInputs(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hash
is not an appropriate variable name, how aboutinputDeviceMap
?const auto
cc3b6d7
to
a04b838
Compare
QHashIterator<SoundDeviceId, AudioInput> it(inputs); | ||
while (it.hasNext()) { | ||
it.next(); | ||
const auto inputDeviceHash = config.getInputs(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conceptually this is still a key/value "map". It doesn't matter if it is a tree or a hash map. This is an implementation detail.
The term "hash" usually means hash code or value and not hash map when used standalone.
If we want to reference the underlying data structure in the naming then inputDeviceMap
.
a04b838
to
03b18ee
Compare
QMultiHash is not a subclass of QHash in Qt6: https://doc.qt.io/qt-6/qtcore-changes-qt6.html#removal-of-qhash-insertmulti so the QHashIterator(const QHash<Key, T> &hash) constructor cannot be used with QMultiHash.
03b18ee
to
b428f48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! LGTM
QMultiHash is not a subclass of QHash in Qt6:
https://doc.qt.io/qt-6/qtcore-changes-qt6.html#removal-of-qhash-insertmulti
so the
QHashIterator(const QHash<Key, T> &hash)
constructor cannot be used with QMultiHash.