cleanup: use taxonomy_set_category() function

Instead of manipulating the taxonomy structures directly, use the
taxonomy_set_category() function. This improves encapsulation and
gives us the possibility to improve the taxonomy data structures.

This concerns three places:
1) git parser
2) XML parser
3) reverse geo-lookup

This improves the XML parser code slightly: The parser assumes that
the value-attribute comes last (after origin and category). While
it still does that, it now at least generates a warning if it encounters
a value-attribute without origin- or category-attribute.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

5 files changed, 32 insertions, 38 deletions

diff --git a/core/divesitehelpers.cpp b/core/divesitehelpers.cpp
index 3f3c4be31..cdc4bcdc3 100644
--- a/core/divesitehelpers.cpp
+++ b/core/divesitehelpers.cpp
@@ -85,18 +85,14 @@ taxonomy_data reverseGeoLookup(degrees_t latitude, degrees_t longitude)
 
 	QString url;
 	QJsonObject obj;
-	taxonomy_data taxonomy = { 0, alloc_taxonomy() };
+	taxonomy_data taxonomy = { 0, 0 };
 
 	// check the oceans API to figure out the body of water
 	url = geonamesOceanURL.arg(getUiLanguage().section(QRegExp("[-_ ]"), 0, 0)).arg(latitude.udeg / 1000000.0).arg(longitude.udeg / 1000000.0);
 	obj = doAsyncRESTGetRequest(url, 5000); // 5 secs. timeout
 	QVariantMap oceanName = obj.value("ocean").toVariant().toMap();
-	if (oceanName["name"].isValid()) {
-		taxonomy.category[0].category = TC_OCEAN;
-		taxonomy.category[0].origin = taxonomy_origin::GEOCODED;
-		taxonomy.category[0].value = copy_qstring(oceanName["name"].toString());
-		taxonomy.nr = 1;
-	}
+	if (oceanName["name"].isValid())
+		taxonomy_set_category(&taxonomy, TC_OCEAN, qPrintable(oceanName["name"].toString()), taxonomy_origin::GEOCODED);
 
 	// check the findNearbyPlaces API from geonames - that should give us country, state, city
 	url = geonamesNearbyPlaceNameURL.arg(getUiLanguage().section(QRegExp("[-_ ]"), 0, 0)).arg(latitude.udeg / 1000000.0).arg(longitude.udeg / 1000000.0);
@@ -114,10 +110,8 @@ taxonomy_data reverseGeoLookup(degrees_t latitude, degrees_t longitude)
 		// fill out all the data - start at COUNTRY since we already got OCEAN above
 		for (int idx = TC_COUNTRY; idx < TC_NR_CATEGORIES; idx++) {
 			if (firstData[taxonomy_api_names[idx]].isValid()) {
-				taxonomy.category[taxonomy.nr].category = idx;
-				taxonomy.category[taxonomy.nr].origin = taxonomy_origin::GEOCODED;
-				taxonomy.category[taxonomy.nr].value = copy_qstring(firstData[taxonomy_api_names[idx]].toString());
-				taxonomy.nr++;
+				QString value = firstData[taxonomy_api_names[idx]].toString();
+				taxonomy_set_category(&taxonomy, (taxonomy_category)idx, qPrintable(value), taxonomy_origin::GEOCODED);
 			}
 		}
 		int l3 = taxonomy_index_for_category(&taxonomy, TC_ADMIN_L3);
@@ -126,10 +120,7 @@ taxonomy_data reverseGeoLookup(degrees_t latitude, degrees_t longitude)
 			// basically this means we did get a local name (what we call town), but just like most places
 			// we didn't get an adminName_3 - which in some regions is the actual city that town belongs to,
 			// then we copy the town into the city
-			taxonomy.category[taxonomy.nr].category = TC_ADMIN_L3;
-			taxonomy.category[taxonomy.nr].origin = taxonomy_origin::GEOCOPIED;
-			taxonomy.category[taxonomy.nr].value = copy_string(taxonomy.category[lt].value);
-			taxonomy.nr++;
+			taxonomy_set_category(&taxonomy, TC_ADMIN_L3, taxonomy.category[lt].value, taxonomy_origin::GEOCOPIED);
 		}
 	} else {
 		report_error("geonames.org did not provide reverse lookup information");
diff --git a/core/load-git.c b/core/load-git.c
index 49482b6c6..444303399 100644
--- a/core/load-git.c
+++ b/core/load-git.c
@@ -309,15 +309,10 @@ static void parse_site_gps(char *line, struct membuffer *str, struct git_parser_
 
 static void parse_site_geo(char *line, struct membuffer *str, struct git_parser_state *state)
 {
-	if (state->active_site->taxonomy.category == NULL)
-		state->active_site->taxonomy.category = alloc_taxonomy();
-	int nr = state->active_site->taxonomy.nr;
-	if (nr < TC_NR_CATEGORIES) {
-		struct taxonomy *t = &state->active_site->taxonomy.category[nr];
-		t->value = detach_cstring(str);
-		sscanf(line, "cat %d origin %d \"", &t->category, (int *)&t->origin);
-		state->active_site->taxonomy.nr++;
-	}
+	int origin;
+	int category;
+	sscanf(line, "cat %d origin %d \"", &category, &origin);
+	taxonomy_set_category(&state->active_site->taxonomy , category, mb_cstring(str), origin);
 }
 
 static char *remove_from_front(struct membuffer *str, int len)
diff --git a/core/parse-xml.c b/core/parse-xml.c
index 0d52eb733..22de449cc 100644
--- a/core/parse-xml.c
+++ b/core/parse-xml.c
@@ -1377,12 +1377,12 @@ static void try_to_fill_trip(dive_trip_t *dive_trip, const char *name, char *buf
 }
 
 /* We're processing a divesite entry - try to fill the components */
-static void try_to_fill_dive_site(struct dive_site *ds, const char *name, char *buf)
+static void try_to_fill_dive_site(struct parser_state *state, const char *name, char *buf)
 {
-	start_match("divesite", name, buf);
+	struct dive_site *ds = state->cur_dive_site;
+	char *taxonomy_value = NULL;
 
-	if (ds->taxonomy.category == NULL)
-		ds->taxonomy.category = alloc_taxonomy();
+	start_match("divesite", name, buf);
 
 	if (MATCH("uuid", hex_value, &ds->uuid))
 		return;
@@ -1394,13 +1394,22 @@ static void try_to_fill_dive_site(struct dive_site *ds, const char *name, char *
 		return;
 	if (MATCH("gps", gps_location, ds))
 		return;
-	if (MATCH("cat.geo", get_index, (int *)&ds->taxonomy.category[ds->taxonomy.nr].category))
+	if (MATCH("cat.geo", get_index, &state->taxonomy_category))
 		return;
-	if (MATCH("origin.geo", get_index, (int *)&ds->taxonomy.category[ds->taxonomy.nr].origin))
+	if (MATCH("origin.geo", get_index, &state->taxonomy_origin))
 		return;
-	if (MATCH("value.geo", utf8_string, &ds->taxonomy.category[ds->taxonomy.nr].value)) {
-		if (ds->taxonomy.nr < TC_NR_CATEGORIES)
-			ds->taxonomy.nr++;
+	if (MATCH("value.geo", utf8_string, &taxonomy_value)) {
+		/* The code assumes that "value.geo" comes last, which is against
+		 * the expectations of an XML file. Let's at least make sure that
+		 * cat and origin have been set! */
+		if (state->taxonomy_category < 0 || state->taxonomy_origin < 0) {
+			report_error("Warning: taxonomy value without origin or category");
+		} else {
+			taxonomy_set_category(&ds->taxonomy, state->taxonomy_category,
+					      taxonomy_value, state->taxonomy_origin);
+		}
+		state->taxonomy_category = state->taxonomy_origin = -1;
+		free(taxonomy_value);
 		return;
 	}
 
@@ -1423,7 +1432,7 @@ static bool entry(const char *name, char *buf, struct parser_state *state)
 		return true;
 	}
 	if (state->cur_dive_site) {
-		try_to_fill_dive_site(state->cur_dive_site, name, buf);
+		try_to_fill_dive_site(state, name, buf);
 		return true;
 	}
 	if (!state->cur_event.deleted) {
diff --git a/core/parse.c b/core/parse.c
index f3b7e0b36..1ddb49d60 100644
--- a/core/parse.c
+++ b/core/parse.c
@@ -203,6 +203,8 @@ void dive_site_start(struct parser_state *state)
 {
 	if (state->cur_dive_site)
 		return;
+	state->taxonomy_category = -1;
+	state->taxonomy_origin = -1;
 	state->cur_dive_site = calloc(1, sizeof(struct dive_site));
 }
 
@@ -210,10 +212,6 @@ void dive_site_end(struct parser_state *state)
 {
 	if (!state->cur_dive_site)
 		return;
-	if (state->cur_dive_site->taxonomy.nr == 0) {
-		free(state->cur_dive_site->taxonomy.category);
-		state->cur_dive_site->taxonomy.category = NULL;
-	}
 	if (state->cur_dive_site->uuid) {
 		struct dive_site *ds = alloc_or_get_dive_site(state->cur_dive_site->uuid, state->sites);
 		merge_dive_site(ds, state->cur_dive_site);
diff --git a/core/parse.h b/core/parse.h
index 350b98ed8..41c1f18ba 100644
--- a/core/parse.h
+++ b/core/parse.h
@@ -52,6 +52,7 @@ struct parser_state {
 	struct sample *cur_sample;		/* non-owning */
 	struct picture cur_picture;		/* owning */
 	char *country, *city;			/* owning */
+	int taxonomy_category, taxonomy_origin;
 
 	bool in_settings;
 	bool in_userid;