Added missing GPX parser error handling

Refactoring

src/gpxparser.cpp

@@ -1,85 +1,33 @@
 #include "gpxparser.h"
 
 
-void GPXParser::handleTrackpointData(DataType type, const QString &value)
+qreal GPXParser::number()
 {
-	switch (type) {
-		case Elevation:
-			_track->last().setElevation(value.toDouble());
-			break;
-		case Time:
-			_track->last().setTimestamp(QDateTime::fromString(value,
-			  Qt::ISODate));
-			break;
-		case Geoidheight:
-			_track->last().setGeoidHeight(value.toDouble());
-			break;
-		case Speed:
-			_track->last().setSpeed(value.toDouble());
-			break;
-		case HeartRate:
-			_track->last().setHeartRate(value.toDouble());
-			break;
-		case Temperature:
-			_track->last().setTemperature(value.toDouble());
-			break;
-		default:
-			break;
-	}
+	bool res;
+	qreal ret = _reader.readElementText().toDouble(&res);
+	if (!res)
+		_reader.raiseError(QString("Invalid %1.").arg(
+		  _reader.name().toString()));
+
+	return ret;
 }
 
-void GPXParser::handleWaypointData(DataType type, const QString &value)
+QDateTime GPXParser::time()
 {
-	switch (type) {
-		case Name:
-			_waypoints.last().setName(value);
-			break;
-		case Description:
-			_waypoints.last().setDescription(value);
-			break;
-		case Time:
-			_waypoints.last().setTimestamp(QDateTime::fromString(value,
-			  Qt::ISODate));
-			break;
-		case Elevation:
-			_waypoints.last().setElevation(value.toDouble());
-			break;
-		case Geoidheight:
-			_waypoints.last().setGeoidHeight(value.toDouble());
-			break;
-		default:
-			break;
-	}
+	QDateTime d = QDateTime::fromString(_reader.readElementText(),
+	  Qt::ISODate);
+	if (!d.isValid())
+		_reader.raiseError(QString("Invalid %1.").arg(
+		  _reader.name().toString()));
+
+	return d;
 }
 
-void GPXParser::handleRoutepointData(DataType type, const QString &value)
-{
-	switch (type) {
-		case Name:
-			_route->last().setName(value);
-			break;
-		case Description:
-			_route->last().setDescription(value);
-			break;
-		case Time:
-			_route->last().setTimestamp(QDateTime::fromString(value,
-			  Qt::ISODate));
-			break;
-		case Elevation:
-			_route->last().setElevation(value.toDouble());
-			break;
-		case Geoidheight:
-			_route->last().setGeoidHeight(value.toDouble());
-			break;
-		default:
-			break;
-	}
-}
-
-Coordinates GPXParser::coordinates(const QXmlStreamAttributes &attr)
+Coordinates GPXParser::coordinates()
 {
 	bool res;
 	qreal lon, lat;
+	const QXmlStreamAttributes &attr = _reader.attributes();
 
 #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)
 	lon = attr.value("lon").toString().toDouble(&res);
@@ -103,28 +51,56 @@ Coordinates GPXParser::coordinates(const QXmlStreamAttributes &attr)
 	return Coordinates(lon, lat);
 }
 
-void GPXParser::handleTrackpointAttributes(const QXmlStreamAttributes &attr)
+void GPXParser::handleTrackpointData(DataType type)
 {
-	_track->last().setCoordinates(coordinates(attr));
+	switch (type) {
+		case Elevation:
+			_track->last().setElevation(number());
+			break;
+		case Time:
+			_track->last().setTimestamp(time());
+			break;
+		case Speed:
+			_track->last().setSpeed(number());
+			break;
+		case HeartRate:
+			_track->last().setHeartRate(number());
+			break;
+		case Temperature:
+			_track->last().setTemperature(number());
+			break;
+		default:
+			break;
+	}
 }
 
-void GPXParser::handleRoutepointAttributes(const QXmlStreamAttributes &attr)
+void GPXParser::handleWaypointData(DataType type, Waypoint &waypoint)
 {
-	_route->last().setCoordinates(coordinates(attr));
-}
-
-void GPXParser::handleWaypointAttributes(const QXmlStreamAttributes &attr)
-{
-	_waypoints.last().setCoordinates(coordinates(attr));
+	switch (type) {
+		case Name:
+			waypoint.setName(_reader.readElementText());
+			break;
+		case Description:
+			waypoint.setDescription(_reader.readElementText());
+			break;
+		case Time:
+			waypoint.setTimestamp(time());
+			break;
+		case Elevation:
+			waypoint.setElevation(number());
+			break;
+		default:
+			break;
+	}
 }
 
 void GPXParser::tpExtension()
 {
 	while (_reader.readNextStartElement()) {
 		if (_reader.name() == "hr")
-			handleTrackpointData(HeartRate, _reader.readElementText());
+			handleTrackpointData(HeartRate);
 		else if (_reader.name() == "atemp")
-			handleTrackpointData(Temperature, _reader.readElementText());
+			handleTrackpointData(Temperature);
 		else
 			_reader.skipCurrentElement();
 	}
@@ -134,11 +110,11 @@ void GPXParser::extensions()
 {
 	while (_reader.readNextStartElement()) {
 		if (_reader.name() == "speed")
-			handleTrackpointData(Speed, _reader.readElementText());
+			handleTrackpointData(Speed);
 		else if (_reader.name() == "hr" || _reader.name() == "heartrate")
-			handleTrackpointData(HeartRate, _reader.readElementText());
+			handleTrackpointData(HeartRate);
 		else if (_reader.name() == "temp")
-			handleTrackpointData(Temperature, _reader.readElementText());
+			handleTrackpointData(Temperature);
 		else if (_reader.name() == "TrackPointExtension")
 			tpExtension();
 		else
@@ -148,62 +124,54 @@ void GPXParser::extensions()
 
 void GPXParser::trackpointData()
 {
+	qreal gh = NAN;
+
 	while (_reader.readNextStartElement()) {
 		if (_reader.name() == "ele")
-			handleTrackpointData(Elevation, _reader.readElementText());
+			handleTrackpointData(Elevation);
 		else if (_reader.name() == "time")
-			handleTrackpointData(Time, _reader.readElementText());
+			handleTrackpointData(Time);
 		else if (_reader.name() == "geoidheight")
-			handleTrackpointData(Geoidheight, _reader.readElementText());
+			gh = number();
 		else if (_reader.name() == "extensions")
 			extensions();
 		else
 			_reader.skipCurrentElement();
 	}
+
+	Trackpoint &t = _track->last();
+	if (!std::isnan(gh) && !std::isnan(t.elevation()))
+		t.setElevation(t.elevation() - gh);
 }
 
-void GPXParser::routepointData()
+void GPXParser::waypointData(Waypoint &waypoint)
 {
+	qreal gh = NAN;
+
 	while (_reader.readNextStartElement()) {
 		if (_reader.name() == "name")
-			handleRoutepointData(Name, _reader.readElementText());
+			handleWaypointData(Name, waypoint);
 		else if (_reader.name() == "desc")
-			handleRoutepointData(Description, _reader.readElementText());
+			handleWaypointData(Description, waypoint);
 		else if (_reader.name() == "ele")
-			handleRoutepointData(Elevation, _reader.readElementText());
+			handleWaypointData(Elevation, waypoint);
 		else if (_reader.name() == "geoidheight")
-			handleRoutepointData(Geoidheight, _reader.readElementText());
+			gh = number();
 		else if (_reader.name() == "time")
-			handleRoutepointData(Time, _reader.readElementText());
+			handleWaypointData(Time, waypoint);
 		else
 			_reader.skipCurrentElement();
 	}
-}
 
-void GPXParser::waypointData()
-{
-	while (_reader.readNextStartElement()) {
-		if (_reader.name() == "name")
-			handleWaypointData(Name, _reader.readElementText());
-		else if (_reader.name() == "desc")
-			handleWaypointData(Description, _reader.readElementText());
-		else if (_reader.name() == "ele")
-			handleWaypointData(Elevation, _reader.readElementText());
-		else if (_reader.name() == "geoidheight")
-			handleWaypointData(Geoidheight, _reader.readElementText());
-		else if (_reader.name() == "time")
-			handleWaypointData(Time, _reader.readElementText());
-		else
-			_reader.skipCurrentElement();
-	}
+	if (!std::isnan(gh) && !std::isnan(waypoint.elevation()))
+		waypoint.setElevation(waypoint.elevation() - gh);
 }
 
 void GPXParser::trackpoints()
 {
 	while (_reader.readNextStartElement()) {
 		if (_reader.name() == "trkpt") {
-			_track->append(Trackpoint());
-			handleTrackpointAttributes(_reader.attributes());
+			_track->append(Trackpoint(coordinates()));
 			trackpointData();
 		} else
 			_reader.skipCurrentElement();
@@ -214,9 +182,8 @@ void GPXParser::routepoints()
 {
 	while (_reader.readNextStartElement()) {
 		if (_reader.name() == "rtept") {
-			_route->append(Waypoint());
-			handleRoutepointAttributes(_reader.attributes());
-			routepointData();
+			_route->append(Waypoint(coordinates()));
+			waypointData(_route->last());
 		} else
 			_reader.skipCurrentElement();
 	}
@@ -244,9 +211,8 @@ void GPXParser::gpx()
 			_route = &_routes.back();
 			routepoints();
 		} else if (_reader.name() == "wpt") {
-			_waypoints.append(Waypoint());
-			handleWaypointAttributes(_reader.attributes());
-			waypointData();
+			_waypoints.append(Waypoint(coordinates()));
+			waypointData(_waypoints.last());
 		} else
 			_reader.skipCurrentElement();
 	}

src/gpxparser.h

@@ -20,8 +20,7 @@ public:
 
 private:
 	enum DataType {
-		Name, Description, Elevation, Time, Geoidheight, Speed, HeartRate,
-		Temperature
+		Name, Description, Elevation, Time, Speed, HeartRate, Temperature
 	};
 
 	bool parse();
@@ -32,17 +31,13 @@ private:
 	void tpExtension();
 	void extensions();
 	void trackpointData();
-	void routepointData();
-	void waypointData();
+	void waypointData(Waypoint &waypoint);
+	qreal number();
+	QDateTime time();
+	Coordinates coordinates();
 
-	Coordinates coordinates(const QXmlStreamAttributes &attr);
-
-	void handleWaypointAttributes(const QXmlStreamAttributes &attr);
-	void handleWaypointData(DataType type, const QString &value);
-	void handleTrackpointAttributes(const QXmlStreamAttributes &attr);
-	void handleTrackpointData(DataType type, const QString &value);
-	void handleRoutepointAttributes(const QXmlStreamAttributes &attr);
-	void handleRoutepointData(DataType type, const QString &value);
+	void handleWaypointData(DataType type, Waypoint &waypoint);
+	void handleTrackpointData(DataType type);
 
 	QXmlStreamReader _reader;
 	QVector<Trackpoint> *_track;

src/route.cpp

@@ -18,7 +18,7 @@ Graph Route::elevation() const
 	for (int i = 0; i < _data.size(); i++)
 		if (_data.at(i).hasElevation())
 			graph.append(GraphPoint(_distance.at(i), NAN,
-			  _data.at(i).elevation() - _data.at(i).geoidHeight()));
+			  _data.at(i).elevation()));
 
 	return graph;
 }

src/track.cpp

@@ -107,7 +107,7 @@ Graph Track::elevation() const
 	for (int i = 0; i < _data.size(); i++)
 		if (_data.at(i).hasElevation())
 			raw.append(GraphPoint(_distance.at(i), _time.at(i),
-			  _data.at(i).elevation() - _data.at(i).geoidHeight()));
+			  _data.at(i).elevation()));
 
 	return filter(raw, WINDOW_EF);
 }

src/trackpoint.cpp

@@ -4,8 +4,8 @@ QDebug operator<<(QDebug dbg, const Trackpoint &trackpoint)
 {
 	dbg.nospace() << "Trackpoint(" << trackpoint.coordinates() << ", "
 	  << trackpoint.timestamp() << ", " << trackpoint.elevation() << ", "
-	  << trackpoint.geoidHeight() << ", " << trackpoint.speed() << ", "
-	  << trackpoint.heartRate() << ", " << trackpoint.temperature() << ")";
+	  << trackpoint.speed() << ", " << trackpoint.heartRate() << ", "
+	  << trackpoint.temperature() << ")";
 
 	return dbg.maybeSpace();
 }

src/trackpoint.h

@@ -9,16 +9,14 @@
 class Trackpoint
 {
 public:
-	Trackpoint() {
-		_elevation = NAN; _geoidHeight = 0; _speed = NAN; _heartRate = NAN;
-		_temperature = NAN;
-	}
-	Trackpoint(const Coordinates &coordinates) {_coordinates = coordinates;}
+	Trackpoint()
+	  {_elevation = NAN; _speed = NAN; _heartRate = NAN; _temperature = NAN;}
+	Trackpoint(const Coordinates &coordinates) : _coordinates(coordinates)
+	  {_elevation = NAN; _speed = NAN; _heartRate = NAN; _temperature = NAN;}
 
 	const Coordinates &coordinates() const {return _coordinates;}
 	const QDateTime &timestamp() const {return _timestamp;}
 	qreal elevation() const {return _elevation;}
-	qreal geoidHeight() const {return _geoidHeight;}
 	qreal speed() const {return _speed;}
 	qreal heartRate() const {return _heartRate;}
 	qreal temperature() const {return _temperature;}
@@ -27,7 +25,6 @@ public:
 	  {_coordinates = coordinates;}
 	void setTimestamp(const QDateTime &timestamp) {_timestamp = timestamp;}
 	void setElevation(qreal elevation) {_elevation = elevation;}
-	void setGeoidHeight(qreal geoidHeight) {_geoidHeight = geoidHeight;}
 	void setSpeed(qreal speed) {_speed = speed;}
 	void setHeartRate(qreal heartRate) {_heartRate = heartRate;}
 	void setTemperature(qreal temperature) {_temperature = temperature;}
@@ -42,7 +39,6 @@ private:
 	Coordinates _coordinates;
 	QDateTime _timestamp;
 	qreal _elevation;
-	qreal _geoidHeight;
 	qreal _speed;
 	qreal _heartRate;
 	qreal _temperature;

src/waypoint.h

@@ -11,16 +11,15 @@
 class Waypoint
 {
 public:
-	Waypoint() {_elevation = NAN; _geoidHeight = 0;}
-	Waypoint(const Coordinates &coordinates)
-	  : _coordinates(coordinates) {_elevation = NAN; _geoidHeight = 0;}
+	Waypoint() {_elevation = NAN;}
+	Waypoint(const Coordinates &coordinates) : _coordinates(coordinates)
+	  {_elevation = NAN;}
 
 	const Coordinates &coordinates() const {return _coordinates;}
 	const QString &name() const {return _name;}
 	const QString &description() const {return _description;}
 	const QDateTime &timestamp() const {return _timestamp;}
 	qreal elevation() const {return _elevation;}
-	qreal geoidHeight() const {return _geoidHeight;}
 
 	void setCoordinates(const Coordinates &coordinates)
 	  {_coordinates = coordinates;}
@@ -29,7 +28,6 @@ public:
 	  {_description = description;}
 	void setTimestamp(const QDateTime &timestamp) {_timestamp = timestamp;}
 	void setElevation(qreal elevation) {_elevation = elevation;}
-	void setGeoidHeight(qreal geoidHeight) {_geoidHeight = geoidHeight;}
 
 	bool hasElevation() const {return !std::isnan(_elevation);}
 
@@ -43,7 +41,6 @@ private:
 	QString _description;
 	QDateTime _timestamp;
 	qreal _elevation;
-	qreal _geoidHeight;
 };
 
 inline uint qHash(const Waypoint &key)

src/waypointitem.cpp

@@ -19,7 +19,7 @@ QString WaypointItem::toolTip()
 	  ::coordinates(_waypoint.coordinates()));
 	if (!std::isnan(_waypoint.elevation()))
 		tt.insert(qApp->translate("WaypointItem", "Elevation"),
-		  ::elevation(_waypoint.elevation() - _waypoint.geoidHeight(), _units));
+		  ::elevation(_waypoint.elevation(), _units));
 	if (!_waypoint.timestamp().isNull())
 		tt.insert(qApp->translate("WaypointItem", "Date"),
 		  _waypoint.timestamp().toString(Qt::SystemLocaleShortDate));