Various modernizations in humminbird. #1100
Conversation
robertlipe
commented
May 6, 2023
robertlipe
commented
- Make humminbird_icons[] a QStringList.
- Various attempts to reduce extra allocations and hope that Qt template magic can
- reduce some of those pointer/len buffer move when deailing with those fixed (not zero-terminated) buffers.
- Try to more clearly mark when arrays represent fixed buffers on device files. I don't think the icon finder ever actually worked the way it was described in the comments.
- Make humminbird_icons[] a QStringList. - Various attempts to reduce extra allocations and hope that Qt template magic can - reduce some of those pointer/len buffer move when deailing with those fixed (not zero-terminated) buffers. - Try to more clearly mark when arrays represent fixed buffers on device files. I don't think the icon finder ever actually worked the way it was described in the comments.
Coverage summary from CodacyMerging #1100 (5bf2c0e) into
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
humminbird.h
Outdated
| @@ -48,7 +49,7 @@ class HumminbirdBase | |||
|
|
|||
| /* Constants */ | |||
|
|
|||
| static constexpr const char* humminbird_icons[] = { | |||
| QStringList humminbird_icons = { | |||
There was a problem hiding this comment.
I would recommend this be a const QStringList. That avoids any checks for detachment as well as alleviates the need to use qAsConst with this list, so you save the two uses you added in humminbird.cc as well.
humminbird.cc
Outdated
| QByteArray ba = QByteArray(w.name, sizeof(w.name)); | ||
| wpt->shortname = QString(ba).remove(QChar::Null); |
There was a problem hiding this comment.
Can this be
auto ba = QByteArray(w.name, qstrnlen(w.name, sizeof(w.name));
?
If so there are 2 more occurrences of this pattern below.
There was a problem hiding this comment.
or better
wpt->shortname = QByteArray(w.name, qstrnlen(w.name, sizeof(w.name));
There was a problem hiding this comment.
Yes! That case of having NO terminator kept haunting me. I wasn't happen with what I landed on (too many type conversions, too many many iterations over the buffer) but this is better.
I actually looked for a qnstrlen but not a qstrnlen. I see now you used this basic approach in garmin.cc where we have that same edge between fixed length, not terminated, buffers and QStrings. Experience!
