core: replace dive_site::dives by an std::vector<>

Since this is now in C++, we don't have to use our crazy
TABLE_* macros.

This contains a logic change: the dives associated to a
dive site are now unsorted.

The old code was subtly buggy: dives were added in a sorted
manner, but when the dive was edited the list was not
resorted. Very unlikely that this leads to a serious
problem, still not good.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2024-05-04 14:41:04 +02:00
parent 8797aa499a
commit c6f9ca8009
12 changed files with 39 additions and 45 deletions

View file

@ -330,7 +330,7 @@ std::vector<const dive_site *> getDiveSitesToExport(bool selectedOnly)
struct dive_site *ds = get_dive_site(i, divelog.sites); struct dive_site *ds = get_dive_site(i, divelog.sites);
if (dive_site_is_empty(ds)) if (dive_site_is_empty(ds))
continue; continue;
if (selectedOnly && !is_dive_site_selected(ds)) if (selectedOnly && !is_dive_site_selected(*ds))
continue; continue;
res.push_back(ds); res.push_back(ds);
} }

View file

@ -23,9 +23,8 @@ static std::vector<dive_site *> addDiveSites(std::vector<std::unique_ptr<dive_si
for (std::unique_ptr<dive_site> &ds: sites) { for (std::unique_ptr<dive_site> &ds: sites) {
// Readd the dives that belonged to this site // Readd the dives that belonged to this site
for (int i = 0; i < ds->dives.nr; ++i) { for (dive *d: ds->dives) {
// TODO: send dive site changed signal // TODO: send dive site changed signal
struct dive *d = ds->dives.dives[i];
d->dive_site = ds.get(); d->dive_site = ds.get();
changedDives.push_back(d); changedDives.push_back(d);
} }
@ -55,8 +54,7 @@ static std::vector<std::unique_ptr<dive_site>> removeDiveSites(std::vector<dive_
for (dive_site *ds: sites) { for (dive_site *ds: sites) {
// Reset the dive_site field of the affected dives // Reset the dive_site field of the affected dives
for (int i = 0; i < ds->dives.nr; ++i) { for (dive *d: ds->dives) {
struct dive *d = ds->dives.dives[i];
d->dive_site = nullptr; d->dive_site = nullptr;
changedDives.push_back(d); changedDives.push_back(d);
} }
@ -157,7 +155,7 @@ PurgeUnusedDiveSites::PurgeUnusedDiveSites()
setText(Command::Base::tr("purge unused dive sites")); setText(Command::Base::tr("purge unused dive sites"));
for (int i = 0; i < divelog.sites->nr; ++i) { for (int i = 0; i < divelog.sites->nr; ++i) {
dive_site *ds = divelog.sites->dive_sites[i]; dive_site *ds = divelog.sites->dive_sites[i];
if (ds->dives.nr == 0) if (ds->dives.empty())
sitesToRemove.push_back(ds); sitesToRemove.push_back(ds);
} }
} }
@ -362,9 +360,9 @@ void MergeDiveSites::redo()
// Add them to the merged-into dive site. Thankfully, we remember // Add them to the merged-into dive site. Thankfully, we remember
// the dives in the sitesToAdd vector. // the dives in the sitesToAdd vector.
for (const std::unique_ptr<dive_site> &site: sitesToAdd) { for (const std::unique_ptr<dive_site> &site: sitesToAdd) {
for (int i = 0; i < site->dives.nr; ++i) { for (dive *d: site->dives) {
add_dive_to_dive_site(site->dives.dives[i], ds); add_dive_to_dive_site(d, ds);
divesChanged.push_back(site->dives.dives[i]); divesChanged.push_back(d);
} }
} }
emit diveListNotifier.divesChanged(divesChanged, DiveField::DIVESITE); emit diveListNotifier.divesChanged(divesChanged, DiveField::DIVESITE);
@ -378,9 +376,9 @@ void MergeDiveSites::undo()
// Before readding the dive sites, unregister the corresponding dives so that they can be // Before readding the dive sites, unregister the corresponding dives so that they can be
// readded to their old dive sites. // readded to their old dive sites.
for (const std::unique_ptr<dive_site> &site: sitesToAdd) { for (const std::unique_ptr<dive_site> &site: sitesToAdd) {
for (int i = 0; i < site->dives.nr; ++i) { for (dive *d: site->dives) {
unregister_dive_from_dive_site(site->dives.dives[i]); unregister_dive_from_dive_site(d);
divesChanged.push_back(site->dives.dives[i]); divesChanged.push_back(d);
} }
} }

View file

@ -1150,7 +1150,7 @@ void process_imported_dives(struct divelog *import_log, int flags,
if (!old_ds) { if (!old_ds) {
/* Dive site doesn't exist. Add it to list of dive sites to be added. */ /* Dive site doesn't exist. Add it to list of dive sites to be added. */
new_ds->dives.nr = 0; /* Caller is responsible for adding dives to site */ new_ds->dives.clear(); /* Caller is responsible for adding dives to site */
add_dive_site_to_table(new_ds, sites_to_add); add_dive_site_to_table(new_ds, sites_to_add);
continue; continue;
} }

View file

@ -180,7 +180,6 @@ dive_site::~dive_site()
free(name); free(name);
free(notes); free(notes);
free(description); free(description);
free(dives.dives);
} }
/* when parsing, dive sites are identified by uuid */ /* when parsing, dive sites are identified by uuid */
@ -199,20 +198,15 @@ struct dive_site *alloc_or_get_dive_site(uint32_t uuid, struct dive_site_table *
return ds; return ds;
} }
int nr_of_dives_at_dive_site(struct dive_site *ds) size_t nr_of_dives_at_dive_site(const dive_site &ds)
{ {
return ds->dives.nr; return ds.dives.size();
} }
bool is_dive_site_selected(struct dive_site *ds) bool is_dive_site_selected(const struct dive_site &ds)
{ {
int i; return any_of(ds.dives.begin(), ds.dives.end(),
[](dive *dive) { return dive->selected; });
for (i = 0; i < ds->dives.nr; i++) {
if (ds->dives.dives[i]->selected)
return true;
}
return false;
} }
int unregister_dive_site(struct dive_site *ds) int unregister_dive_site(struct dive_site *ds)
@ -353,7 +347,6 @@ void purge_empty_dive_sites(struct dive_site_table *ds_table)
void add_dive_to_dive_site(struct dive *d, struct dive_site *ds) void add_dive_to_dive_site(struct dive *d, struct dive_site *ds)
{ {
int idx;
if (!d) { if (!d) {
report_info("Warning: add_dive_to_dive_site called with NULL dive"); report_info("Warning: add_dive_to_dive_site called with NULL dive");
return; return;
@ -368,8 +361,7 @@ void add_dive_to_dive_site(struct dive *d, struct dive_site *ds)
report_info("Warning: adding dive that already belongs to a dive site to a different site"); report_info("Warning: adding dive that already belongs to a dive site to a different site");
unregister_dive_from_dive_site(d); unregister_dive_from_dive_site(d);
} }
idx = dive_table_get_insertion_index(&ds->dives, d); ds->dives.push_back(d);
add_to_dive_table(&ds->dives, idx, d);
d->dive_site = ds; d->dive_site = ds;
} }
@ -377,8 +369,12 @@ struct dive_site *unregister_dive_from_dive_site(struct dive *d)
{ {
struct dive_site *ds = d->dive_site; struct dive_site *ds = d->dive_site;
if (!ds) if (!ds)
return NULL; return nullptr;
remove_dive(d, &ds->dives); auto it = std::find(ds->dives.begin(), ds->dives.end(), d);
d->dive_site = NULL; if (it != ds->dives.end())
ds->dives.erase(it);
else
report_info("Warning: dive not found in divesite table, even though it should be registered there.");
d->dive_site = nullptr;
return ds; return ds;
} }

View file

@ -15,7 +15,7 @@ struct dive_site
{ {
uint32_t uuid = 0; uint32_t uuid = 0;
char *name = nullptr; char *name = nullptr;
struct dive_table dives = { 0, 0, nullptr }; std::vector<dive *> dives;
location_t location = { { 9 }, { 0 } }; location_t location = { { 9 }, { 0 } };
char *description = nullptr; char *description = nullptr;
char *notes = nullptr; char *notes = nullptr;
@ -50,8 +50,8 @@ void sort_dive_site_table(struct dive_site_table *ds_table);
int add_dive_site_to_table(struct dive_site *ds, struct dive_site_table *ds_table); int add_dive_site_to_table(struct dive_site *ds, struct dive_site_table *ds_table);
struct dive_site *alloc_or_get_dive_site(uint32_t uuid, struct dive_site_table *ds_table); struct dive_site *alloc_or_get_dive_site(uint32_t uuid, struct dive_site_table *ds_table);
struct dive_site *alloc_dive_site(); struct dive_site *alloc_dive_site();
int nr_of_dives_at_dive_site(struct dive_site *ds); size_t nr_of_dives_at_dive_site(const struct dive_site &ds);
bool is_dive_site_selected(struct dive_site *ds); bool is_dive_site_selected(const struct dive_site &ds);
int unregister_dive_site(struct dive_site *ds); int unregister_dive_site(struct dive_site *ds);
int register_dive_site(struct dive_site *ds); int register_dive_site(struct dive_site *ds);
void delete_dive_site(struct dive_site *ds, struct dive_site_table *ds_table); void delete_dive_site(struct dive_site *ds, struct dive_site_table *ds_table);

View file

@ -707,7 +707,7 @@ static void save_dives_buffer(struct membuffer *b, bool select_only, bool anonym
if (dive_site_is_empty(ds)) if (dive_site_is_empty(ds))
continue; continue;
/* Only write used dive sites when exporting selected dives */ /* Only write used dive sites when exporting selected dives */
if (select_only && !is_dive_site_selected(ds)) if (select_only && !is_dive_site_selected(*ds))
continue; continue;
put_format(b, "<site uuid='%8x'", ds->uuid); put_format(b, "<site uuid='%8x'", ds->uuid);

View file

@ -62,9 +62,9 @@ void DiveSiteListView::diveSiteClicked(const QModelIndex &index)
MainWindow::instance()->editDiveSite(ds); MainWindow::instance()->editDiveSite(ds);
break; break;
case LocationInformationModel::REMOVE: case LocationInformationModel::REMOVE:
if (ds->dives.nr > 0 && if (!ds->dives.empty() &&
QMessageBox::warning(this, tr("Delete dive site?"), QMessageBox::warning(this, tr("Delete dive site?"),
tr("This dive site has %n dive(s). Do you really want to delete it?\n", "", ds->dives.nr), tr("This dive site has %n dive(s). Do you really want to delete it?\n", "", ds->dives.size()),
QMessageBox::Yes|QMessageBox::No) == QMessageBox::No) QMessageBox::Yes|QMessageBox::No) == QMessageBox::No)
return; return;
Command::deleteDiveSites(QVector<dive_site *>{ds}); Command::deleteDiveSites(QVector<dive_site *>{ds});

View file

@ -1410,7 +1410,7 @@ void MainWindow::on_actionImportDiveSites_triggered()
} }
// The imported dive sites still have pointers to imported dives - remove them // The imported dive sites still have pointers to imported dives - remove them
for (int i = 0; i < log.sites->nr; ++i) for (int i = 0; i < log.sites->nr; ++i)
log.sites->dive_sites[i]->dives.nr = 0; log.sites->dive_sites[i]->dives.clear();
QString source = fileNames.size() == 1 ? fileNames[0] : tr("multiple files"); QString source = fileNames.size() == 1 ? fileNames[0] : tr("multiple files");

View file

@ -487,7 +487,7 @@ void LocationFilterDelegate::paint(QPainter *painter, const QStyleOptionViewItem
} else { } else {
int distanceMeters = get_distance(&ds->location, &currentLocation); int distanceMeters = get_distance(&ds->location, &currentLocation);
QString distance = distance_string(distanceMeters); QString distance = distance_string(distanceMeters);
int nr = nr_of_dives_at_dive_site(ds); size_t nr = nr_of_dives_at_dive_site(*ds);
bottomText += tr(" (~%1 away").arg(distance); bottomText += tr(" (~%1 away").arg(distance);
bottomText += tr(", %n dive(s) here)", "", nr); bottomText += tr(", %n dive(s) here)", "", nr);
} }

View file

@ -92,7 +92,7 @@ QVariant LocationInformationModel::getDiveSiteData(const struct dive_site *ds, i
switch(column) { switch(column) {
case DIVESITE: return QVariant::fromValue<dive_site *>((dive_site *)ds); // Not nice: casting away const case DIVESITE: return QVariant::fromValue<dive_site *>((dive_site *)ds); // Not nice: casting away const
case NAME: return QString(ds->name); case NAME: return QString(ds->name);
case NUM_DIVES: return ds->dives.nr; case NUM_DIVES: return static_cast<int>(ds->dives.size());
case LOCATION: return "TODO"; case LOCATION: return "TODO";
case DESCRIPTION: return QString(ds->description); case DESCRIPTION: return QString(ds->description);
case NOTES: return QString(ds->name); case NOTES: return QString(ds->name);
@ -207,7 +207,7 @@ bool DiveSiteSortedModel::lessThan(const QModelIndex &i1, const QModelIndex &i2)
QString::localeAwareCompare(QString(ds1->name), QString(ds2->name)) < 0; // TODO: avoid copy QString::localeAwareCompare(QString(ds1->name), QString(ds2->name)) < 0; // TODO: avoid copy
} }
case LocationInformationModel::NUM_DIVES: { case LocationInformationModel::NUM_DIVES: {
int cmp = ds1->dives.nr - ds2->dives.nr; int cmp = static_cast<int>(ds1->dives.size()) - static_cast<int>(ds2->dives.size());
// Since by default nr dives is descending, invert sort direction of names, such that // Since by default nr dives is descending, invert sort direction of names, such that
// the names are listed as ascending. // the names are listed as ascending.
return cmp != 0 ? cmp < 0 : return cmp != 0 ? cmp < 0 :

View file

@ -1199,10 +1199,10 @@ void DiveTripModelTree::divesDeletedInternal(dive_trip *trip, bool deleteTrip, c
static QVector<dive *> getDivesForSite(struct dive_site *ds) static QVector<dive *> getDivesForSite(struct dive_site *ds)
{ {
QVector<dive *> diveSiteDives; QVector<dive *> diveSiteDives;
diveSiteDives.reserve(ds->dives.nr); diveSiteDives.reserve(ds->dives.size());
for (int i = 0; i < ds->dives.nr; ++i) for (dive *d: ds->dives)
diveSiteDives.push_back(ds->dives.dives[i]); diveSiteDives.push_back(d);
return diveSiteDives; return diveSiteDives;
} }

View file

@ -120,13 +120,13 @@ const QVector<dive_site *> &MapLocationModel::selectedDs() const
static bool hasVisibleDive(const dive_site *ds) static bool hasVisibleDive(const dive_site *ds)
{ {
return std::any_of(&ds->dives.dives[0], &ds->dives.dives[ds->dives.nr], return std::any_of(ds->dives.begin(), ds->dives.end(),
[] (const dive *d) { return !d->hidden_by_filter; }); [] (const dive *d) { return !d->hidden_by_filter; });
} }
static bool hasSelectedDive(const dive_site *ds) static bool hasSelectedDive(const dive_site *ds)
{ {
return std::any_of(&ds->dives.dives[0], &ds->dives.dives[ds->dives.nr], return std::any_of(ds->dives.begin(), ds->dives.end(),
[] (const dive *d) { return d->selected; }); [] (const dive *d) { return d->selected; });
} }