Fixed defects found in the review of MapRouteItem

 - Reviewed by Jussi Laitinen

 - Created enum for MapScene item zValues

diff --git a/src/map/friendgroupitem.cpp b/src/map/friendgroupitem.cpp
index 83318de..18a1c82 100644 (file)
--- a/src/map/friendgroupitem.cpp
+++ b/src/map/friendgroupitem.cpp
@@ -39,7 +39,7 @@ FriendGroupItem::FriendGroupItem(FriendLocationItem *item)
     setPixmap(QPixmap(":/res/images/friend_group.png"));
 
     setOffset(-pixmap().width() / 2, -pixmap().height() / 2);
-    setZValue(FRIEND_LOCATION_ICON_Z_LEVEL);
+    setZValue(FriendLocationItemZValue);
 
     joinFriend(item);
     setPos(item->pos());

diff --git a/src/map/friendlocationitem.cpp b/src/map/friendlocationitem.cpp
index 3e1c201..e5a492f 100644 (file)
--- a/src/map/friendlocationitem.cpp
+++ b/src/map/friendlocationitem.cpp
@@ -37,7 +37,7 @@ FriendLocationItem::FriendLocationItem(const QString &userId,
     qDebug() << __PRETTY_FUNCTION__;
 
     setPos(UNDEFINED, UNDEFINED);
-    setZValue(FRIEND_LOCATION_ICON_Z_LEVEL);
+    setZValue(FriendLocationItemZValue);
 }
 
 bool FriendLocationItem::isPartOfGroup() const

diff --git a/src/map/gpslocationitem.cpp b/src/map/gpslocationitem.cpp
index 1369dfe..e8023fb 100644 (file)
--- a/src/map/gpslocationitem.cpp
+++ b/src/map/gpslocationitem.cpp
@@ -35,7 +35,7 @@ GPSLocationItem::GPSLocationItem()
     qDebug() << __PRETTY_FUNCTION__;
 
     setPos(QPoint(UNDEFINED, UNDEFINED));
-    setZValue(OWN_LOCATION_ICON_Z_LEVEL);
+    setZValue(GPSLocationItemZValue);
 
     // create a child item for the actual red position spot
     m_pixmapItem = new QGraphicsPixmapItem(QPixmap(":/res/images/gps_position.png"));

diff --git a/src/map/mapcommon.h b/src/map/mapcommon.h
index f779c02..b229be3 100644 (file)
--- a/src/map/mapcommon.h
+++ b/src/map/mapcommon.h
@@ -25,7 +25,6 @@
 
 #include <QtCore>
 
-#include "coordinates/geocoordinate.h"
 #include "osm.h"
 
 const int MAP_TILE_MIN_INDEX = 0; ///< First index number of map tiles
@@ -63,12 +62,6 @@ const int MAP_DEFAULT_ZOOM_LEVEL = MAP_VIEW_MIN_ZOOM_LEVEL;
 const qreal MAP_DEFAULT_LONGITUDE = 0.0000;  ///< Default longitude value
 const qreal MAP_DEFAULT_LATITUDE = 0.0000; ///< Default latitude value
 
-/**
-* @var FRIEND_LOCATION_ICON_Z_LEVEL
-* @brief layer of friend location icon
-*/
-const int FRIEND_LOCATION_ICON_Z_LEVEL = MAP_SCENE_MIN_NORMAL_LEVEL + OSM_MAX_ZOOM_LEVEL + 1;
-
 const int GROUP_ITEM_FRIENDS_COUNT_X = 13;  ///< Group item friends count x value
 const int GROUP_ITEM_FRIENDS_COUNT_Y = 13;  ///< Group item friends count y value
 const int GROUP_ITEM_FRIENDS_COUNT_WIDTH = 17;  ///< Group item friends count width value
@@ -76,12 +69,6 @@ const int GROUP_ITEM_FRIENDS_COUNT_HEIGHT = 17; ///< Group item friends count he
 
 const int PRESS_MANHATTAN_LENGTH = 30;   ///< Friend/group item press manhattan length
 
-/**
-* @var OWN_LOCATION_ICON_Z_LEVEL
-* @brief layer of own location icon
-*/
-const int OWN_LOCATION_ICON_Z_LEVEL = FRIEND_LOCATION_ICON_Z_LEVEL + 1;
-
 const double MIN_LONGITUDE = -180.0;  ///< Minimum longitude value
 const double MAX_LONGITUDE = 180.0;  ///< Maximum longitude value
 
@@ -106,4 +93,15 @@ const QString ERROR_VALUE_NOT_FOUND_ON_SETTINGS = "Value_not_found";
 */
 const int UNDEFINED = INT_MIN;
 
+/**
+* @enum SceneItemZValues
+*
+* @brief Z level values for different MapScene items
+*/
+enum SceneItemZValues {
+    RouteItemZValue = MAP_SCENE_MIN_NORMAL_LEVEL + OSM_MAX_ZOOM_LEVEL + 1,
+    FriendLocationItemZValue,
+    GPSLocationItemZValue
+};
+
 #endif // MAPCOMMON_H

diff --git a/src/map/mapengine.cpp b/src/map/mapengine.cpp
index 5eb4bca..d3935a9 100644 (file)
--- a/src/map/mapengine.cpp
+++ b/src/map/mapengine.cpp
@@ -142,8 +142,11 @@ void MapEngine::centerAndZoomTo(QRect rect)
     // fit inside the usable view area
     int shift = 0;
     while ((rect.height() > (viewUsableHeight * (1 << shift)))
-           || (rect.width() > (viewUsableWidth * (1 << shift))))
+           || (rect.width() > (viewUsableWidth * (1 << shift)))) {
+
         shift++;
+    }
+
 
     scrollToPosition(SceneCoordinate(double(rect.center().x()), double(rect.center().y())));
 
@@ -430,22 +433,11 @@ void MapEngine::setRoute(Route &route)
 
     m_route = route;
 
-    qDebug() << __PRETTY_FUNCTION__ << "from:" << m_route.startPointName();
-    qDebug() << __PRETTY_FUNCTION__ << "to:" << m_route.endPointName();
-    qDebug() << __PRETTY_FUNCTION__ << "distance:" << m_route.totalDistance();
-    qDebug() << __PRETTY_FUNCTION__ << "estimated time:" << m_route.totalTime();
-
-    foreach (GeoCoordinate point, m_route.geometryPoints())
-        qDebug() << __PRETTY_FUNCTION__ << "geometry point:" << point;
-
-    foreach (RouteSegment segment, m_route.segments()) {
-        qDebug() << __PRETTY_FUNCTION__ << "segment:" << segment.instruction();
-    }
-
     // delete old route track (if exists)
     if (m_mapRouteItem) {
         m_mapScene->removeItem(m_mapRouteItem);
         delete m_mapRouteItem;
+        m_mapRouteItem = 0;
     }
 
     // create new route track

diff --git a/src/map/maprouteitem.cpp b/src/map/maprouteitem.cpp
index 0427d4d..9e3e3ab 100644 (file)
--- a/src/map/maprouteitem.cpp
+++ b/src/map/maprouteitem.cpp
@@ -22,35 +22,44 @@
 
 #include <QPen>
 
-#include "map/mapcommon.h"
-#include "map/mapengine.h"
-#include "routing/route.h"
 #include "coordinates/geocoordinate.h"
 #include "coordinates/scenecoordinate.h"
+#include "map/mapcommon.h"
+#include "routing/route.h"
 
 #include "maprouteitem.h"
 
-MapRouteItem::MapRouteItem(QGraphicsItem *parent) :
-    QGraphicsItemGroup(parent)
+MapRouteItem::MapRouteItem(QGraphicsItem *parent)
+    : QGraphicsItemGroup(parent)
 {
-    setZValue(100);
+    qDebug() << __PRETTY_FUNCTION__;
+
+    setZValue(RouteItemZValue);
 }
 
-MapRouteItem::MapRouteItem(Route *route, QGraphicsItem *parent) :
-        QGraphicsItemGroup(parent)
+MapRouteItem::MapRouteItem(Route *route, QGraphicsItem *parent)
+    : QGraphicsItemGroup(parent)
 {
-    setZValue(100);
+    qDebug() << __PRETTY_FUNCTION__;
+
+    setZValue(RouteItemZValue);
     setRoute(route);
 }
 
 void MapRouteItem::clear()
 {
-    foreach(QGraphicsItem *item, childItems())
+    qDebug() << __PRETTY_FUNCTION__;
+
+    foreach(QGraphicsItem *item, childItems()) {
         removeFromGroup(item);
+        delete item;
+    }
 }
 
 void MapRouteItem::setRoute(Route *route)
 {
+    qDebug() << __PRETTY_FUNCTION__;
+
     const int LINE_WIDTH = 5;
     const QColor LINE_COLOR = Qt::red;
 

diff --git a/src/map/maprouteitem.h b/src/map/maprouteitem.h
index 1586419..48ced74 100644 (file)
--- a/src/map/maprouteitem.h
+++ b/src/map/maprouteitem.h
@@ -57,7 +57,7 @@ public:
  ******************************************************************************/
 public:
     /**
-    * @brief Clear a route
+    * @brief Clear the route
     *
     * Deletes all line items.
     */
@@ -66,7 +66,7 @@ public:
     /**
     * @brief Set route
     *
-    * Does clear old route before setting new one. Does create lines presenting a track.
+    * Does clear old route before setting the new one. Does create lines presenting a track.
     *
     * @param route Route information used for building the lines presenting a track
     */

diff --git a/src/map/mapscene.cpp b/src/map/mapscene.cpp
index 3c74c60..2712dbc 100644 (file)
--- a/src/map/mapscene.cpp
+++ b/src/map/mapscene.cpp
@@ -22,6 +22,7 @@
 #include <QDebug>
 
 #include "coordinates/scenecoordinate.h"
+#include "coordinates/geocoordinate.h"
 #include "mapcommon.h"
 #include "maptile.h"
 

diff --git a/src/map/osm.h b/src/map/osm.h
index 66a2b64..8dc78ba 100644 (file)
--- a/src/map/osm.h
+++ b/src/map/osm.h
@@ -38,7 +38,6 @@ const int OSM_TILES_PER_SIDE = (1 << OSM_MAX_ZOOM_LEVEL);   ///< Amount of tiles
 * @brief Maximum latitude value ( = arctan(sinh(pi))
 */
 const double OSM_MAX_LATITUDE = 85.05112877980659237802;
-const double OSM_MIN_LATITUDE = -OSM_MAX_LATITUDE;          ///< Minimum latitude value
 
 // MAP PIXELS
 /**

diff --git a/src/map/ownlocationitem.cpp b/src/map/ownlocationitem.cpp
index f138c3a..e86f02e 100644 (file)
--- a/src/map/ownlocationitem.cpp
+++ b/src/map/ownlocationitem.cpp
@@ -35,6 +35,6 @@ OwnLocationItem::OwnLocationItem()
     setPixmap(ownLocationIcon);
 
     setPos(QPoint(UNDEFINED, UNDEFINED));
-    setZValue(OWN_LOCATION_ICON_Z_LEVEL);
+    setZValue(GPSLocationItemZValue);
     setOffset(-ownLocationIcon.width() / 2, OFFSET_TO_ARROW_TIP);
 }

diff --git a/tests/coordinates/geocoordinate/testgeocoordinate.cpp b/tests/coordinates/geocoordinate/testgeocoordinate.cpp
index 9acbbe6..cb8f479 100644 (file)
--- a/tests/coordinates/geocoordinate/testgeocoordinate.cpp
+++ b/tests/coordinates/geocoordinate/testgeocoordinate.cpp
@@ -99,7 +99,7 @@ void TestGeoCoordinate::conversion_data()
                                                          - ONE_SCENE_PIXEL_WIDTH_IN_DEGREES;
 
     QTest::newRow("bottom right") << SceneCoordinate(OSM_MAP_MAX_PIXEL_X, OSM_MAP_MAX_PIXEL_Y)
-                                  << GeoCoordinate(OSM_MIN_LATITUDE,
+                                  << GeoCoordinate(-OSM_MAX_LATITUDE,
                                                    LAST_SCENE_HORIZONTAL_PIXEL_LONGITUDE);
 }
 

diff --git a/tests/coordinates/scenecoordinate/testscenecoordinate.cpp b/tests/coordinates/scenecoordinate/testscenecoordinate.cpp
index 764f981..dac47bb 100644 (file)
--- a/tests/coordinates/scenecoordinate/testscenecoordinate.cpp
+++ b/tests/coordinates/scenecoordinate/testscenecoordinate.cpp
@@ -93,15 +93,15 @@ void TestSceneCoordinate::conversion_data()
     const double LAST_SCENE_HORIZONTAL_PIXEL_LONGITUDE = MAX_LONGITUDE
                                                          - ONE_SCENE_PIXEL_WIDTH_IN_DEGREES;
     QTest::newRow("bottom right pixel")
-            << GeoCoordinate(OSM_MIN_LATITUDE, LAST_SCENE_HORIZONTAL_PIXEL_LONGITUDE)
+            << GeoCoordinate(-OSM_MAX_LATITUDE, LAST_SCENE_HORIZONTAL_PIXEL_LONGITUDE)
             << SceneCoordinate(OSM_MAP_MAX_PIXEL_X, OSM_MAP_MAX_PIXEL_Y);
 
     QTest::newRow("southeast corner with 180 degrees longitude")
-            << GeoCoordinate(OSM_MIN_LATITUDE, MAX_LONGITUDE)
+            << GeoCoordinate(-OSM_MAX_LATITUDE, MAX_LONGITUDE)
             << SceneCoordinate(OSM_MAP_MIN_PIXEL_X, OSM_MAP_MAX_PIXEL_Y);
 
     QTest::newRow("southeast corner just little over west edge of the map")
-            << GeoCoordinate(OSM_MIN_LATITUDE, MIN_LONGITUDE - ONE_SCENE_PIXEL_WIDTH_IN_DEGREES)
+            << GeoCoordinate(-OSM_MAX_LATITUDE, MIN_LONGITUDE - ONE_SCENE_PIXEL_WIDTH_IN_DEGREES)
             << SceneCoordinate(OSM_MAP_MAX_PIXEL_X, OSM_MAP_MAX_PIXEL_Y);
 }
 

diff --git a/tests/map/friendgroupitem/testfriendgroupitem.cpp b/tests/map/friendgroupitem/testfriendgroupitem.cpp
index 5b14d02..a891b06 100644 (file)
--- a/tests/map/friendgroupitem/testfriendgroupitem.cpp
+++ b/tests/map/friendgroupitem/testfriendgroupitem.cpp
@@ -131,7 +131,7 @@ void TestFriendGroupItem::constructor()
     QCOMPARE(group->pos(), QPointF(100, 100));
 
     // zValue should be set
-    QCOMPARE(group->zValue(), static_cast<qreal>(FRIEND_LOCATION_ICON_Z_LEVEL));
+    QCOMPARE(group->zValue(), static_cast<qreal>(FriendLocationItemZValue));
 
     // icon offset should be set
     QCOMPARE(group->offset(), QPointF(-pixmap->width() / 2, -pixmap->height() / 2));

diff --git a/tests/map/friendlocationitem/testfriendlocationitem.cpp b/tests/map/friendlocationitem/testfriendlocationitem.cpp
index 444d98b..a33cdc8 100644 (file)
--- a/tests/map/friendlocationitem/testfriendlocationitem.cpp
+++ b/tests/map/friendlocationitem/testfriendlocationitem.cpp
@@ -74,7 +74,7 @@ void TestFriendLocationItem::constructor()
     FriendLocationItem friendLocation(userID);
 
     // Test Z-value
-    QCOMPARE(static_cast<int>(friendLocation.zValue()), FRIEND_LOCATION_ICON_Z_LEVEL);
+    QCOMPARE(static_cast<int>(friendLocation.zValue()), static_cast<int>(FriendLocationItemZValue));
 
     // Test ItemIgnoresTransformations Flags
     QGraphicsItem::GraphicsItemFlags friendLocationItemFlags = friendLocation.flags();

diff --git a/tests/map/gpslocationitem/testgpslocationitem.cpp b/tests/map/gpslocationitem/testgpslocationitem.cpp
index c4f1cc1..2e6283d 100644 (file)
--- a/tests/map/gpslocationitem/testgpslocationitem.cpp
+++ b/tests/map/gpslocationitem/testgpslocationitem.cpp
@@ -60,7 +60,7 @@ void TestGPSLocationItem::constructor()
     QCOMPARE(item.pos(), QPointF(UNDEFINED, UNDEFINED));
 
     // zValue should be set
-    QCOMPARE(item.zValue(), static_cast<qreal>(OWN_LOCATION_ICON_Z_LEVEL));
+    QCOMPARE(item.zValue(), static_cast<qreal>(GPSLocationItemZValue));
 
     // by default the item should be hidden
     QVERIFY(item.isVisible() == false);

diff --git a/tests/map/ownlocationitem/testownlocationitem.cpp b/tests/map/ownlocationitem/testownlocationitem.cpp
index 137d883..0d54702 100644 (file)
--- a/tests/map/ownlocationitem/testownlocationitem.cpp
+++ b/tests/map/ownlocationitem/testownlocationitem.cpp
@@ -77,7 +77,7 @@ class TestOwnLocationItem: public QObject
      QCOMPARE(ownLocationItem.pos(), defaultLocationPoint);
 
      // Test Z-value
-     QCOMPARE(static_cast<int>(ownLocationItem.zValue()), OWN_LOCATION_ICON_Z_LEVEL);
+     QCOMPARE(static_cast<int>(ownLocationItem.zValue()), static_cast<int>(GPSLocationItemZValue));
 
      // Test Offset
      QCOMPARE(ownLocationItem.offset(),