From 32df0ab0daa8551e24d93082ad182673bbf37cdf Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 14 Oct 2018 08:56:53 +0200 Subject: Cleanup: remove DiveItem and TripItem classes The DiveItem and TripItem classes were wrappers around dive * and dive_trip * used to extract tabular data. With the rework of DiveTripModel they lost all their state besides the pointer itself. The usage was: DiveItem item(d); item.data(...); This can now be simplified to the much more idiomatic diveData(d, ...); and analoguously for TripItem. While adapting the data() function to be part of DiveTripModel, change the QVariant ret switch(...) { ... case ...: ret = ...; break; ... } return ret; style to switch(...) { ... case ...: return ...; } Not only is this shorter and easier to reason about, it generally also improves the generated code. The compiler can directly construct the return value in the buffer provided by the caller. Though modern compilers start to be very good at avoiding unnecessary copies. In total this cleanup results in a net-reduction of 190 lines of code. Signed-off-by: Berthold Stoeger --- qt-models/divetripmodel.cpp | 533 ++++++++++++++++---------------------------- qt-models/divetripmodel.h | 57 +---- 2 files changed, 200 insertions(+), 390 deletions(-) diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index 94f4b6b51..ff96669a7 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -9,7 +9,7 @@ #include #include -static int nitrox_sort_value(struct dive *dive) +static int nitrox_sort_value(const struct dive *dive) { int o2, he, o2max; get_dive_gas(dive, &o2, &he, &o2max); @@ -18,7 +18,6 @@ static int nitrox_sort_value(struct dive *dive) static QVariant dive_table_alignment(int column) { - QVariant retVal; switch (column) { case DiveTripModel::DEPTH: case DiveTripModel::DURATION: @@ -28,9 +27,8 @@ static QVariant dive_table_alignment(int column) case DiveTripModel::OTU: case DiveTripModel::MAXCNS: // Right align numeric columns - retVal = int(Qt::AlignRight | Qt::AlignVCenter); - break; - // NR needs to be left aligned becase its the indent marker for trips too + return int(Qt::AlignRight | Qt::AlignVCenter); + // NR needs to be left aligned because its the indent marker for trips too case DiveTripModel::NR: case DiveTripModel::DATE: case DiveTripModel::RATING: @@ -42,21 +40,19 @@ static QVariant dive_table_alignment(int column) case DiveTripModel::COUNTRY: case DiveTripModel::BUDDIES: case DiveTripModel::LOCATION: - retVal = int(Qt::AlignLeft | Qt::AlignVCenter); - break; + return int(Qt::AlignLeft | Qt::AlignVCenter); } - return retVal; + return QVariant(); } -QVariant TripItem::data(int column, int role) const +QVariant DiveTripModel::tripData(const dive_trip *trip, int column, int role) { - QVariant ret; bool oneDayTrip=true; - if (role == DiveTripModel::TRIP_ROLE) - return QVariant::fromValue(trip); + if (role == TRIP_ROLE) + return QVariant::fromValue((void *)trip); // Not nice: casting away a const - if (role == DiveTripModel::SORT_ROLE) + if (role == SORT_ROLE) return (qulonglong)trip->when; if (role == Qt::DisplayRole) { @@ -74,14 +70,13 @@ QVariant TripItem::data(int column, int role) const if (countShown < trip->nrdives) shownText = tr("(%1 shown)").arg(countShown); if (!empty_string(trip->location)) - ret = QString(trip->location) + ", " + get_trip_date_string(trip->when, trip->nrdives, oneDayTrip) + " "+ shownText; + return QString(trip->location) + ", " + get_trip_date_string(trip->when, trip->nrdives, oneDayTrip) + " "+ shownText; else - ret = get_trip_date_string(trip->when, trip->nrdives, oneDayTrip) + shownText; - break; + return get_trip_date_string(trip->when, trip->nrdives, oneDayTrip) + shownText; } } - return ret; + return QVariant(); } static const QString icon_names[4] = { @@ -91,148 +86,162 @@ static const QString icon_names[4] = { QStringLiteral(":photo-in-out-icon") }; -QVariant DiveItem::data(int column, int role) const +static int countPhotos(const struct dive *d) +{ // Determine whether dive has pictures, and whether they were taken during or before/after dive. + const int bufperiod = 120; // A 2-min buffer period. Photos within 2 min of dive are assumed as + int diveTotaltime = dive_endtime(d) - d->when; // taken during the dive, not before/after. + int pic_offset, icon_index = 0; + FOR_EACH_PICTURE (d) { // Step through each of the pictures for this dive: + pic_offset = picture->offset.seconds; + if ((pic_offset < -bufperiod) | (pic_offset > diveTotaltime+bufperiod)) { + icon_index |= 0x02; // If picture is before/after the dive + } // then set the appropriate bit ... + else { + icon_index |= 0x01; // else set the bit for picture during the dive + } + } + return icon_index; // return value: 0=no pictures; 1=pictures during dive; +} // 2=pictures before/after; 3=pictures during as well as before/after + +static QString displayDuration(const struct dive *d) { - QVariant retVal; + if (prefs.units.show_units_table) + return get_dive_duration_string(d->duration.seconds, gettextFromC::tr("h"), gettextFromC::tr("min"), "", ":", d->dc.divemode == FREEDIVE); + else + return get_dive_duration_string(d->duration.seconds, "", "", "", ":", d->dc.divemode == FREEDIVE); +} +static QString displayTemperature(const struct dive *d, bool units) +{ + if (!d->watertemp.mkelvin) + return QString(); + return get_temperature_string(d->watertemp, units); +} + +static QString displaySac(const struct dive *d, bool units) +{ + if (!d->sac) + return QString(); + QString s = get_volume_string(d->sac, units); + return units ? s + gettextFromC::tr("/min") : s; +} + +static QString displayWeight(const struct dive *d, bool units) +{ + QString s = weight_string(total_weight(d)); + if (!units) + return s; + else if (get_units()->weight == units::KG) + return s + gettextFromC::tr("kg"); + else + return s + gettextFromC::tr("lbs"); +} + +QVariant DiveTripModel::diveData(const struct dive *d, int column, int role) +{ switch (role) { case Qt::TextAlignmentRole: - retVal = dive_table_alignment(column); - break; - case DiveTripModel::SORT_ROLE: + return dive_table_alignment(column); + case SORT_ROLE: switch (column) { case NR: - retVal = (qlonglong)d->when; - break; + return (qlonglong)d->when; case DATE: - retVal = (qlonglong)d->when; - break; + return (qlonglong)d->when; case RATING: - retVal = d->rating; - break; + return d->rating; case DEPTH: - retVal = d->maxdepth.mm; - break; + return d->maxdepth.mm; case DURATION: - retVal = d->duration.seconds; - break; + return d->duration.seconds; case TEMPERATURE: - retVal = d->watertemp.mkelvin; - break; + return d->watertemp.mkelvin; case TOTALWEIGHT: - retVal = total_weight(d); - break; + return total_weight(d); case SUIT: - retVal = QString(d->suit); - break; + return QString(d->suit); case CYLINDER: - retVal = QString(d->cylinder[0].type.description); - break; + return QString(d->cylinder[0].type.description); case GAS: - retVal = nitrox_sort_value(d); - break; + return nitrox_sort_value(d); case SAC: - retVal = d->sac; - break; + return d->sac; case OTU: - retVal = d->otu; - break; + return d->otu; case MAXCNS: - retVal = d->maxcns; - break; + return d->maxcns; case TAGS: - retVal = displayTags(); - break; + return get_taglist_string(d->tag_list); case PHOTOS: - retVal = countPhotos(); - break; + return countPhotos(d); case COUNTRY: - retVal = QString(get_dive_country(d)); - break; + return QString(get_dive_country(d)); case BUDDIES: - retVal = QString(d->buddy); - break; + return QString(d->buddy); case LOCATION: - retVal = QString(get_dive_location(d)); - break; + return QString(get_dive_location(d)); } break; case Qt::DisplayRole: switch (column) { case NR: - retVal = d->number; - break; + return d->number; case DATE: - retVal = displayDate(); - break; + return get_dive_date_string(d->when); case DEPTH: - retVal = prefs.units.show_units_table ? displayDepthWithUnit() : displayDepth(); - break; + return get_depth_string(d->maxdepth, prefs.units.show_units_table); case DURATION: - retVal = displayDuration(); - break; + return displayDuration(d); case TEMPERATURE: - retVal = prefs.units.show_units_table ? retVal = displayTemperatureWithUnit() : displayTemperature(); - break; + return displayTemperature(d, prefs.units.show_units_table); case TOTALWEIGHT: - retVal = prefs.units.show_units_table ? retVal = displayWeightWithUnit() : displayWeight(); - break; + return displayWeight(d, prefs.units.show_units_table); case SUIT: - retVal = QString(d->suit); - break; + return QString(d->suit); case CYLINDER: - retVal = QString(d->cylinder[0].type.description); - break; + return QString(d->cylinder[0].type.description); case SAC: - retVal = prefs.units.show_units_table ? retVal = displaySacWithUnit() : displaySac(); - break; + return displaySac(d, prefs.units.show_units_table); case OTU: - retVal = d->otu; - break; + return d->otu; case MAXCNS: if (prefs.units.show_units_table) - retVal = QString("%1%").arg(d->maxcns); + return QString("%1%").arg(d->maxcns); else - retVal = d->maxcns; - break; + return d->maxcns; case TAGS: - retVal = displayTags(); - break; + return get_taglist_string(d->tag_list); case PHOTOS: break; case COUNTRY: - retVal = QString(get_dive_country(d)); - break; + return QString(get_dive_country(d)); case BUDDIES: - retVal = QString(d->buddy); - break; + return QString(d->buddy); case LOCATION: - retVal = QString(get_dive_location(d)); - break; + return QString(get_dive_location(d)); case GAS: - const char *gas_string = get_dive_gas_string(d); - retVal = QString(gas_string); - free((void*)gas_string); - break; + char *gas_string = get_dive_gas_string(d); + QString ret(gas_string); + free(gas_string); + return ret; } break; case Qt::DecorationRole: switch (column) { //TODO: ADD A FLAG case COUNTRY: - retVal = QVariant(); - break; + return QVariant(); case LOCATION: if (dive_has_gps_location(d)) { IconMetrics im = defaultIconMetrics(); - retVal = QIcon(":globe-icon").pixmap(im.sz_small, im.sz_small); + return QIcon(":globe-icon").pixmap(im.sz_small, im.sz_small); } break; case PHOTOS: if (d->picture_list) { IconMetrics im = defaultIconMetrics(); - retVal = QIcon(icon_names[countPhotos()]).pixmap(im.sz_small, im.sz_small); + return QIcon(icon_names[countPhotos(d)]).pixmap(im.sz_small, im.sz_small); } // If there are photos, show one of the three photo icons: fish= photos during dive; break; // sun=photos before/after dive; sun+fish=photos during dive as well as before/after } @@ -240,188 +249,55 @@ QVariant DiveItem::data(int column, int role) const case Qt::ToolTipRole: switch (column) { case NR: - retVal = tr("#"); - break; + return tr("#"); case DATE: - retVal = tr("Date"); - break; + return tr("Date"); case RATING: - retVal = tr("Rating"); - break; + return tr("Rating"); case DEPTH: - retVal = tr("Depth(%1)").arg((get_units()->length == units::METERS) ? tr("m") : tr("ft")); - break; + return tr("Depth(%1)").arg((get_units()->length == units::METERS) ? tr("m") : tr("ft")); case DURATION: - retVal = tr("Duration"); - break; + return tr("Duration"); case TEMPERATURE: - retVal = tr("Temp.(%1%2)").arg(UTF8_DEGREE).arg((get_units()->temperature == units::CELSIUS) ? "C" : "F"); - break; + return tr("Temp.(%1%2)").arg(UTF8_DEGREE).arg((get_units()->temperature == units::CELSIUS) ? "C" : "F"); case TOTALWEIGHT: - retVal = tr("Weight(%1)").arg((get_units()->weight == units::KG) ? tr("kg") : tr("lbs")); - break; + return tr("Weight(%1)").arg((get_units()->weight == units::KG) ? tr("kg") : tr("lbs")); case SUIT: - retVal = tr("Suit"); - break; + return tr("Suit"); case CYLINDER: - retVal = tr("Cylinder"); - break; + return tr("Cylinder"); case GAS: - retVal = tr("Gas"); - break; + return tr("Gas"); case SAC: const char *unit; get_volume_units(0, NULL, &unit); - retVal = tr("SAC(%1)").arg(QString(unit).append(tr("/min"))); - break; + return tr("SAC(%1)").arg(QString(unit).append(tr("/min"))); case OTU: - retVal = tr("OTU"); - break; + return tr("OTU"); case MAXCNS: - retVal = tr("Max. CNS"); - break; + return tr("Max. CNS"); case TAGS: - retVal = tr("Tags"); - break; + return tr("Tags"); case PHOTOS: - retVal = tr("Media before/during/after dive"); - break; + return tr("Media before/during/after dive"); case COUNTRY: - retVal = tr("Country"); - break; + return tr("Country"); case BUDDIES: - retVal = tr("Buddy"); - break; + return tr("Buddy"); case LOCATION: - retVal = tr("Location"); - break; + return tr("Location"); } break; - case DiveTripModel::STAR_ROLE: - retVal = d->rating; - break; - case DiveTripModel::DIVE_ROLE: - retVal = QVariant::fromValue(d); - break; - case DiveTripModel::DIVE_IDX: - retVal = get_divenr(d); - break; - case DiveTripModel::SELECTED_ROLE: - retVal = d->selected; - break; - } - return retVal; -} - -bool DiveItem::setData(const QModelIndex &index, const QVariant &value, int role) -{ - if (role != Qt::EditRole) - return false; - if (index.column() != NR) - return false; - - int v = value.toInt(); - if (v == 0) - return false; - - int i; - struct dive *dive; - for_each_dive (i, dive) { - if (dive->number == v) - return false; - } - d->number = value.toInt(); - mark_divelist_changed(true); - return true; -} - -QString DiveItem::displayDate() const -{ - return get_dive_date_string(d->when); -} - -QString DiveItem::displayDepth() const -{ - return get_depth_string(d->maxdepth); -} - -QString DiveItem::displayDepthWithUnit() const -{ - return get_depth_string(d->maxdepth, true); -} - -int DiveItem::countPhotos() const -{ // Determine whether dive has pictures, and whether they were taken during or before/after dive. - const int bufperiod = 120; // A 2-min buffer period. Photos within 2 min of dive are assumed as - int diveTotaltime = dive_endtime(d) - d->when; // taken during the dive, not before/after. - int pic_offset, icon_index = 0; - FOR_EACH_PICTURE (d) { // Step through each of the pictures for this dive: - pic_offset = picture->offset.seconds; - if ((pic_offset < -bufperiod) | (pic_offset > diveTotaltime+bufperiod)) { - icon_index |= 0x02; // If picture is before/after the dive - } // then set the appropriate bit ... - else { - icon_index |= 0x01; // else set the bit for picture during the dive - } + case STAR_ROLE: + return d->rating; + case DIVE_ROLE: + return QVariant::fromValue((void *)d); // Not nice: casting away a const + case DIVE_IDX: + return get_divenr(d); + case SELECTED_ROLE: + return d->selected; } - return icon_index; // return value: 0=no pictures; 1=pictures during dive; -} // 2=pictures before/after; 3=pictures during as well as before/after - -QString DiveItem::displayDuration() const -{ - if (prefs.units.show_units_table) - return get_dive_duration_string(d->duration.seconds, tr("h"), tr("min"), "", ":", d->dc.divemode == FREEDIVE); - else - return get_dive_duration_string(d->duration.seconds, "", "", "", ":", d->dc.divemode == FREEDIVE); -} - -QString DiveItem::displayTemperature() const -{ - if (!d->watertemp.mkelvin) - return QString(); - return get_temperature_string(d->watertemp, false); -} - -QString DiveItem::displayTemperatureWithUnit() const -{ - if (!d->watertemp.mkelvin) - return QString(); - return get_temperature_string(d->watertemp, true); -} - -QString DiveItem::displaySac() const -{ - if (!d->sac) - return QString(); - return get_volume_string(d->sac, false); -} - -QString DiveItem::displaySacWithUnit() const -{ - if (!d->sac) - return QString(); - return get_volume_string(d->sac, true).append(tr("/min")); -} - -QString DiveItem::displayWeight() const -{ - return weight_string(weight()); -} - -QString DiveItem::displayWeightWithUnit() const -{ - return weight_string(weight()) + ((get_units()->weight == units::KG) ? tr("kg") : tr("lbs")); -} - -QString DiveItem::displayTags() const -{ - return get_taglist_string(d->tag_list); -} - -int DiveItem::weight() const -{ - weight_t tw = { total_weight(d) }; - return tw.grams; + return QVariant(); } DiveTripModel *DiveTripModel::instance() @@ -508,135 +384,97 @@ Qt::ItemFlags DiveTripModel::flags(const QModelIndex &index) const QVariant DiveTripModel::headerData(int section, Qt::Orientation orientation, int role) const { - QVariant ret; if (orientation == Qt::Vertical) - return ret; + return QVariant(); switch (role) { case Qt::TextAlignmentRole: - ret = dive_table_alignment(section); - break; + return dive_table_alignment(section); case Qt::FontRole: - ret = defaultModelFont(); - break; + return defaultModelFont(); case Qt::DisplayRole: switch (section) { case NR: - ret = tr("#"); - break; + return tr("#"); case DATE: - ret = tr("Date"); - break; + return tr("Date"); case RATING: - ret = tr("Rating"); - break; + return tr("Rating"); case DEPTH: - ret = tr("Depth"); - break; + return tr("Depth"); case DURATION: - ret = tr("Duration"); - break; + return tr("Duration"); case TEMPERATURE: - ret = tr("Temp."); - break; + return tr("Temp."); case TOTALWEIGHT: - ret = tr("Weight"); - break; + return tr("Weight"); case SUIT: - ret = tr("Suit"); - break; + return tr("Suit"); case CYLINDER: - ret = tr("Cylinder"); - break; + return tr("Cylinder"); case GAS: - ret = tr("Gas"); - break; + return tr("Gas"); case SAC: - ret = tr("SAC"); - break; + return tr("SAC"); case OTU: - ret = tr("OTU"); - break; + return tr("OTU"); case MAXCNS: - ret = tr("Max CNS"); - break; + return tr("Max CNS"); case TAGS: - ret = tr("Tags"); - break; + return tr("Tags"); case PHOTOS: - ret = tr("Media"); - break; + return tr("Media"); case COUNTRY: - ret = tr("Country"); - break; + return tr("Country"); case BUDDIES: - ret = tr("Buddy"); - break; + return tr("Buddy"); case LOCATION: - ret = tr("Location"); - break; + return tr("Location"); } break; case Qt::ToolTipRole: switch (section) { case NR: - ret = tr("#"); - break; + return tr("#"); case DATE: - ret = tr("Date"); - break; + return tr("Date"); case RATING: - ret = tr("Rating"); - break; + return tr("Rating"); case DEPTH: - ret = tr("Depth(%1)").arg((get_units()->length == units::METERS) ? tr("m") : tr("ft")); - break; + return tr("Depth(%1)").arg((get_units()->length == units::METERS) ? tr("m") : tr("ft")); case DURATION: - ret = tr("Duration"); - break; + return tr("Duration"); case TEMPERATURE: - ret = tr("Temp.(%1%2)").arg(UTF8_DEGREE).arg((get_units()->temperature == units::CELSIUS) ? "C" : "F"); - break; + return tr("Temp.(%1%2)").arg(UTF8_DEGREE).arg((get_units()->temperature == units::CELSIUS) ? "C" : "F"); case TOTALWEIGHT: - ret = tr("Weight(%1)").arg((get_units()->weight == units::KG) ? tr("kg") : tr("lbs")); - break; + return tr("Weight(%1)").arg((get_units()->weight == units::KG) ? tr("kg") : tr("lbs")); case SUIT: - ret = tr("Suit"); - break; + return tr("Suit"); case CYLINDER: - ret = tr("Cylinder"); - break; + return tr("Cylinder"); case GAS: - ret = tr("Gas"); - break; + return tr("Gas"); case SAC: const char *unit; get_volume_units(0, NULL, &unit); - ret = tr("SAC(%1)").arg(QString(unit).append(tr("/min"))); - break; + return tr("SAC(%1)").arg(QString(unit).append(tr("/min"))); case OTU: - ret = tr("OTU"); - break; + return tr("OTU"); case MAXCNS: - ret = tr("Max CNS"); - break; + return tr("Max CNS"); case TAGS: - ret = tr("Tags"); - break; + return tr("Tags"); case PHOTOS: - ret = tr("Media before/during/after dive"); - break; + return tr("Media before/during/after dive"); case BUDDIES: - ret = tr("Buddy"); - break; + return tr("Buddy"); case LOCATION: - ret = tr("Location"); - break; + return tr("Location"); } break; } - return ret; + return QVariant(); } DiveTripModel::Item::Item(dive_trip *t, const QVector &divesIn) : trip(t), @@ -834,17 +672,38 @@ QVariant DiveTripModel::data(const QModelIndex &index, int role) const auto entry = tripOrDive(index); if (entry.first) - return TripItem(entry.first).data(index.column(), role); + return tripData(entry.first, index.column(), role); else if (entry.second) - return DiveItem(entry.second).data(index.column(), role); + return diveData(entry.second, index.column(), role); else return QVariant(); } bool DiveTripModel::setData(const QModelIndex &index, const QVariant &value, int role) { + // We only support setting of data for dives and there, only the number. dive *d = diveOrNull(index); - return d ? DiveItem(d).setData(index, value, role) : false; + if (!d) + return false; + if (role != Qt::EditRole) + return false; + if (index.column() != NR) + return false; + + int v = value.toInt(); + if (v == 0) + return false; + + // Only accept numbers that are not already in use by other dives. + int i; + struct dive *dive; + for_each_dive (i, dive) { + if (dive->number == v) + return false; + } + d->number = v; + mark_divelist_changed(true); + return true; } int DiveTripModel::findTripIdx(const dive_trip *trip) const diff --git a/qt-models/divetripmodel.h b/qt-models/divetripmodel.h index fb3c3395d..d75926f9e 100644 --- a/qt-models/divetripmodel.h +++ b/qt-models/divetripmodel.h @@ -4,59 +4,6 @@ #include "core/dive.h" #include -#include // For Q_DECLARE_TR_FUNCTIONS - -struct DiveItem { - Q_DECLARE_TR_FUNCTIONS(TripItem) // Is that TripItem on purpose? -public: - enum Column { - NR, - DATE, - RATING, - DEPTH, - DURATION, - TEMPERATURE, - TOTALWEIGHT, - SUIT, - CYLINDER, - GAS, - SAC, - OTU, - MAXCNS, - TAGS, - PHOTOS, - BUDDIES, - COUNTRY, - LOCATION, - COLUMNS - }; - - QVariant data(int column, int role) const; - dive *d; - DiveItem(dive *dIn) : d(dIn) {} // Trivial constructor - bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::EditRole); - QString displayDate() const; - QString displayDuration() const; - QString displayDepth() const; - QString displayDepthWithUnit() const; - QString displayTemperature() const; - QString displayTemperatureWithUnit() const; - QString displayWeight() const; - QString displayWeightWithUnit() const; - QString displaySac() const; - QString displaySacWithUnit() const; - QString displayTags() const; - int countPhotos() const; - int weight() const; -}; - -struct TripItem { - Q_DECLARE_TR_FUNCTIONS(TripItem) -public: - QVariant data(int column, int role) const; - dive_trip_t *trip; - TripItem(dive_trip_t *tIn) : trip(tIn) {} // Trivial constructor -}; class DiveTripModel : public QAbstractItemModel { Q_OBJECT @@ -165,6 +112,10 @@ private: int findDiveInTrip(int tripIdx, const dive *d) const; // Find dive inside trip. Second parameter is index of trip int findInsertionIndex(timestamp_t when) const; // Where to insert item with timestamp "when" + // Access trip and dive data + static QVariant diveData(const struct dive *d, int column, int role); + static QVariant tripData(const dive_trip *trip, int column, int role); + // Select or deselect dives void changeDiveSelection(dive_trip *trip, const QVector &dives, bool select); -- cgit v1.2.3-70-g09d2