Fixing defects found in the review of Route, RouteSegment & RoutingService
authorSami Rämö <sami.ramo@ixonos.com>
Thu, 5 Aug 2010 12:30:23 +0000 (15:30 +0300)
committerSami Rämö <sami.ramo@ixonos.com>
Thu, 5 Aug 2010 12:30:23 +0000 (15:30 +0300)
 - Reviewed by Pekka Nissinen

src/routing/route.cpp
src/routing/route.h
src/routing/routesegment.cpp
src/routing/routesegment.h
src/routing/routingservice.cpp
src/routing/routingservice.h
tests/routing/routesegment/testroutesegment.cpp

index 7213149..3e4ae8d 100644 (file)
@@ -32,14 +32,14 @@ Route::Route()
     qDebug() << __PRETTY_FUNCTION__;
 }
 
-void Route::appendGeometryPoint(GeoCoordinate geometryPoint)
+void Route::appendGeometryPoint(const GeoCoordinate &geometryPoint)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
     m_geometryPoints.append(geometryPoint);
 }
 
-void Route::appendSegment(RouteSegment &segment)
+void Route::appendSegment(const RouteSegment &segment)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
@@ -67,14 +67,14 @@ const QList<RouteSegment>& Route::segments() const
     return m_segments;
 }
 
-void Route::setEndPointName(QString endPoint)
+void Route::setEndPointName(const QString &endPoint)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
     m_endPointName = endPoint;
 }
 
-void Route::setStartPointName(QString startPoint)
+void Route::setStartPointName(const QString &startPoint)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
index 76ac3dc..eff9a0c 100644 (file)
@@ -45,6 +45,9 @@ public:
     */
     Route();
 
+/*******************************************************************************
+ * MEMBER FUNCTIONS AND SLOTS
+ ******************************************************************************/
     /**
     * @brief Append geometry point (a.k.a track point) of the route
     *
@@ -52,17 +55,16 @@ public:
     *
     * @param geometryPoint Geometry point
     */
-    void appendGeometryPoint(GeoCoordinate geometryPoint);
+    void appendGeometryPoint(const GeoCoordinate &geometryPoint);
 
     /**
     * @brief Append a route segment to the route
     *
-    * Takes the ownership of the segment object. Appending must be done in order starting from the
-    * beginning of the route.
+    * Appending must be done in order starting from the beginning of the route.
     *
     * @param segment Route segment object
     */
-    void appendSegment(RouteSegment &segment);
+    void appendSegment(const RouteSegment &segment);
 
     /**
     * @brief Getter for route end point name
@@ -90,14 +92,14 @@ public:
     *
     * @param endPoint Name of the end point
     */
-    void setEndPointName(QString endPoint);
+    void setEndPointName(const QString &endPoint);
 
     /**
     * @brief Set name of the route start point
     *
     * @param startPoint Name of the route start point
     */
-    void setStartPointName(QString startPoint);
+    void setStartPointName(const QString &startPoint);
 
     /**
     * @brief Set total distance of the route
@@ -134,6 +136,9 @@ public:
     */
     int totalTime() const;
 
+/*******************************************************************************
+ * DATA MEMBERS
+ ******************************************************************************/
 private:
     int m_totalDistance;                ///< route total distance in meters
     int m_totalTime;                    ///< estimated route total time in seconds
index 79bd520..f3961cc 100644 (file)
@@ -85,14 +85,14 @@ void RouteSegment::setAzimuth(qreal azimuth)
     m_azimuth = azimuth;
 }
 
-void RouteSegment::setEarthDirection(QString direction)
+void RouteSegment::setEarthDirection(const QString &direction)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
     m_earthDirection = direction;
 }
 
-void RouteSegment::setInstruction(QString instruction)
+void RouteSegment::setInstruction(const QString &instruction)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
@@ -106,7 +106,7 @@ void RouteSegment::setLength(qreal meters)
     m_length = meters;
 }
 
-void RouteSegment::setLengthCaption(QString length)
+void RouteSegment::setLengthCaption(const QString &length)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
@@ -134,7 +134,7 @@ void RouteSegment::setTurnAngle(qreal degrees)
     m_turnAngle = degrees;
 }
 
-void RouteSegment::setTurnType(QString type)
+void RouteSegment::setTurnType(const QString &type)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
@@ -145,6 +145,17 @@ QString RouteSegment::street() const
 {
     qDebug() << __PRETTY_FUNCTION__;
 
+    // turn type codes
+    const QString TURN_TYPE_CONTINUE = "C";
+    const QString TURN_TYPE_LEFT = "TL";
+    const QString TURN_TYPE_SLIGHT_LEFT = "TSLL";
+    const QString TURN_TYPE_SHARP_LEFT = "TSHL";
+    const QString TURN_TYPE_RIGHT = "TR";
+    const QString TURN_TYPE_SLIGHT_RIGHT = "TSLR";
+    const QString TURN_TYPE_SHARP_RIGHT = "TSHR";
+    const QString TURN_TYPE_U_TURN = "TU";
+    const QString TURN_TYPE_ROUNDABOUT = "EXIT";
+
     // regular expressions for matching/replacing instructions
     const QString REGEXP_1ST_SEGMENT
         = "^Head (north|northeast|east|southeast|south|southwest|west|northwest)";
@@ -164,31 +175,31 @@ QString RouteSegment::street() const
         QRegExp regexp(REGEXP_1ST_SEGMENT);
         result.replace(regexp, "");
 
-    } else if (m_turnType == "C") {
+    } else if (m_turnType == TURN_TYPE_CONTINUE) {
         // continue straight
         QRegExp regexp(REGEXP_CONTINUE);
         result.replace(regexp, "");
 
-    } else if ((m_turnType == "TL") || (m_turnType == "TSLL")
-               || (m_turnType == "TSHL") || (m_turnType == "TR")
-               || (m_turnType == "TSLR") || (m_turnType == "TSHR")) {
+    } else if ((m_turnType == TURN_TYPE_LEFT) || (m_turnType == TURN_TYPE_SLIGHT_LEFT)
+               || (m_turnType == TURN_TYPE_SHARP_LEFT) || (m_turnType == TURN_TYPE_RIGHT)
+               || (m_turnType == TURN_TYPE_SLIGHT_RIGHT) || (m_turnType == TURN_TYPE_SHARP_RIGHT)) {
         // turns
         QRegExp regexp(REGEXP_TURNS);
         result.replace(regexp, "");
 
-    } else if (m_turnType == "TU") {
+    } else if (m_turnType == TURN_TYPE_U_TURN) {
         // u-turn
         QRegExp regexp(REGEXP_U_TURN);
         result.replace(regexp, "");
 
-    } else if (m_turnType.startsWith("EXIT")) {
+    } else if (m_turnType.startsWith(TURN_TYPE_ROUNDABOUT)) {
         // roundabout
         QRegExp regexp(REGEXP_ROUNDABOUT);
         result.replace(regexp, "");
 
     } else {
-        // no match, parsing failed
-        qCritical() << __PRETTY_FUNCTION__ << "PARSING FAILED! ( turn type:"
+        // no match, unknown turn type
+        qCritical() << __PRETTY_FUNCTION__ << "UNKNOWN TURN TYPE CODE! ( turn type:"
                     << m_turnType << "instruction:" << m_instruction << ")";
     }
 
index 11c69b9..105a9d7 100644 (file)
@@ -33,7 +33,6 @@
  *
  * @author Sami Rämö - sami.ramo@ixonos.com
  */
-
 class RouteSegment
 {
 public:
@@ -42,6 +41,9 @@ public:
     */
     RouteSegment();
 
+/*******************************************************************************
+ * MEMBER FUNCTIONS AND SLOTS
+ ******************************************************************************/
     /**
     * @brief Getter for azimuth
     *
@@ -100,14 +102,14 @@ public:
     *
     * @param direction Direction code
     */
-    void setEarthDirection(QString direction);
+    void setEarthDirection(const QString &direction);
 
     /**
     * @brief Set instruction text
     *
     * @param instruction Instructon text
     */
-    void setInstruction(QString instruction);
+    void setInstruction(const QString &instruction);
 
     /**
     * @brief Set length
@@ -121,7 +123,7 @@ public:
     *
     * @param length Length caption text
     */
-    void setLengthCaption(QString length);
+    void setLengthCaption(const QString &length);
 
     /**
     * @brief Set position index
@@ -149,7 +151,7 @@ public:
     *
     * @param type Turn type code
     */
-    void setTurnType(QString type);
+    void setTurnType(const QString &type);
 
     /**
     * @brief Get street name/number parsed from the text instruction
@@ -189,6 +191,9 @@ public:
     */
     const QString& turnType() const;
 
+/*******************************************************************************
+ * DATA MEMBERS
+ ******************************************************************************/
 private:
     int m_timeSeconds;          ///< estimated time required to travel the segment in seconds
     int m_positionIndex;        ///< index of the first point of the segment in route geometry
@@ -201,7 +206,6 @@ private:
     QString m_instruction;      ///< text instruction, e.g. Turn left at Oxford Street
     QString m_lengthCaption;    ///< length of the segment e.g. 22m, 23.4 km, 14.4 miles
     QString m_turnType;         ///< code of the turn type, optional, absent for the first segment
-
 };
 
 #endif // ROUTESEGMENT_H
index 8a536a3..bf0f26e 100644 (file)
@@ -162,6 +162,7 @@ void RoutingService::parseSearchResults(const QByteArray &jsonReply)
 
                 m_searchResults.append(location);
             }
+
             emit locationDataParsed(m_searchResults);
         }
     }
@@ -231,7 +232,7 @@ void RoutingService::requestFinished(QNetworkReply *reply)
     }
 }
 
-void RoutingService::requestLocation(QString location)
+void RoutingService::requestLocation(const QString &location)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
@@ -244,7 +245,7 @@ void RoutingService::requestLocation(QString location)
     sendRequest(url);
 }
 
-void RoutingService::requestRoute(GeoCoordinate from, GeoCoordinate to)
+void RoutingService::requestRoute(const GeoCoordinate &from, const GeoCoordinate &to)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
index d7ab0f3..d2c1fc2 100644 (file)
 #include "location.h"
 #include "route.h"
 
-class NetworkAccessManager;
 class QNetworkReply;
 class QNetworkRequest;
 class QUrl;
 
 class GeoCoordinate;
+class NetworkAccessManager;
 
 /**
 * @brief RoutingService class for communicating with CloudMade server
@@ -47,7 +47,6 @@ class RoutingService : public QObject
     Q_OBJECT
 
 public:
-
     /**
     * @brief Default constructor
     *
@@ -59,7 +58,6 @@ public:
  * MEMBER FUNCTIONS AND SLOTS
  ******************************************************************************/
 public slots:
-
     /**
     * @brief Public slot, which indicates when http request has been completed
     *
@@ -73,17 +71,16 @@ public slots:
     * @param from Start point of the route
     * @param to End point of the route
     */
-    void requestRoute(GeoCoordinate from, GeoCoordinate to);
+    void requestRoute(const GeoCoordinate &from, const GeoCoordinate &to);
 
     /**
     * @brief Request location information from the server
     *
     * @param location location (address, city etc.)
     */
-    void requestLocation(QString location);
+    void requestLocation(const QString &location);
 
 private:
-
     /**
     * @brief Parses routing data from JSON string
     *
@@ -115,7 +112,6 @@ private:
  * SIGNALS
  ******************************************************************************/
 signals:
-
     /**
     * @brief Signals error
     *
@@ -142,7 +138,6 @@ signals:
  * DATA MEMBERS
  ******************************************************************************/
 private:
-
     QList<QNetworkReply *> m_currentRequests;   ///< List of current http requests
     QList<Location> m_searchResults;            ///< List of search results
 
index 4571b88..27cc507 100644 (file)
@@ -46,7 +46,6 @@ void TestRouteSegment::settersAndGetters()
     QString direction = "NW";
     QString instructionText = "Turn left at Oxford Street";
     QString lengthCaption = "123 m";
-    QString streetName = "Oxford Street";
     QString turnType = "TL";
 
     RouteSegment segment;
@@ -129,6 +128,12 @@ void TestRouteSegment::streetNameParser_data()
     QTest::newRow("roundabout, 5th exit")
             << "EXIT5" << "At the roundabout, take the 5th exit onto 4;9;13;23"
             << "4;9;13;23";
+
+    QTest::newRow("unknown turn type code, IGNORE THIS MESSAGE WHEN RUNNING THE UNIT TEST")
+            << "ABCD123" << "At the roundabout, take the 5th exit onto 4;9;13;23"
+            << "At the roundabout, take the 5th exit onto 4;9;13;23";
+
+
 }
 
 QTEST_APPLESS_MAIN(TestRouteSegment);