Code review. Sergey should check @todo issues.

map/compass_arrow.hpp

@@ -33,11 +33,17 @@ private:
   yg::Color m_southColor;
   double m_angle;
 
+  /// @todo It's a scoped_ptr. You don't pass it outside the class.
   shared_ptr<yg::gl::DisplayList> m_displayList;
+
+  /// @todo Am I missed something? Where does this ptr initialized?
   shared_ptr<RotateScreenTask> m_rotateScreenTask;
 
   mutable vector<m2::AnyRectD> m_boundRects;
 
+  /// @todo I don't think that it's a good idea to store matrix as state of the class.
+  /// Just calculate it in the needed plase (draw()). Otherwise you should not forget
+  /// to recalculate it. Now it depends on pivot(), but what happens when pivot is changed?
   math::Matrix<double, 3, 3> m_drawM;
 
   Framework * m_framework;

map/framework.cpp

@@ -135,6 +135,9 @@ Framework::Framework()
     m_queryMaxScaleMode(false),
     m_drawPlacemark(false),
     //m_hasPendingShowRectFixed(false),
+
+    /// @todo Probably we can make this like static const int.
+    /// It's not a class state, so no need to store it in memory.
     m_metresMinWidth(10),
     m_metresMaxWidth(1000000),
 #if defined(OMIM_OS_MAC) || defined(OMIM_OS_WINDOWS) || defined(OMIM_OS_LINUX)
@@ -142,6 +145,7 @@ Framework::Framework()
 #else
     m_minRulerWidth(60),
 #endif
+
     m_width(0),
     m_height(0),
     m_informationDisplay(this),

map/location_state.cpp

@@ -43,10 +43,14 @@ namespace location
     m_compassBorderColor = p.m_compassBorderColor;
     m_useDirectionArrow = p.m_useDirectionArrow;
     m_framework = p.m_framework;
+
+    /// @todo Probably we can make this like static const int.
+    /// It's not a class state, so no need to store it in memory.
     m_arrowScale = 0.7;
     m_arrowWidth = 40 * m_arrowScale;
     m_arrowHeight = 50 * m_arrowScale;
     m_arrowBackHeight = 10 * m_arrowScale;
+
     m_boundRects.resize(1);
 
     setColor(EActive, yg::Color(0x2f, 0xb5, 0xea, 128));
@@ -132,10 +136,8 @@ namespace location
 
   void State::OnGpsUpdate(location::GpsInfo const & info)
   {
-    double lon = info.m_longitude;
-    double lat = info.m_latitude;
-
-    m2::RectD rect = MercatorBounds::MetresToXY(lon, lat, info.m_horizontalAccuracy);
+    m2::RectD rect = MercatorBounds::MetresToXY(
+          info.m_longitude, info.m_latitude, info.m_horizontalAccuracy);
     m2::PointD const center = rect.Center();
 
     m_hasPosition = true;
@@ -165,9 +167,9 @@ namespace location
       if (setScale != -1)
         rect = scales::GetRectForLevel(setScale, center, 1.0);
 
-      double a = m_framework->GetNavigator().Screen().GetAngle();
-      double dx = rect.SizeX();
-      double dy = rect.SizeY();
+      double const a = m_framework->GetNavigator().Screen().GetAngle();
+      double const dx = rect.SizeX();
+      double const dy = rect.SizeY();
 
       m_framework->ShowRectFixed(m2::AnyRectD(rect.Center(), a, m2::RectD(-dx/2, -dy/2, dx/2, dy/2)));
 
@@ -185,7 +187,6 @@ namespace location
       SetIsCentered(true);
       CheckCompassRotation();
       CheckFollowCompass();
-
       break;
 
     case ELocationSkipCentering:

map/location_state.hpp

@@ -53,15 +53,14 @@ namespace location
   {
   private:
 
-    double m_errorRadius; //< error radius in mercator
-    m2::PointD m_position; //< position in mercator
+    double m_errorRadius;   //< error radius in mercator
+    m2::PointD m_position;  //< position in mercator
 
     CompassFilter m_compassFilter;
     double m_drawHeading;
 
     bool m_hasPosition;
     bool m_hasCompass;
-
     bool m_isCentered;
 
     ELocationProcessMode m_locationProcessMode;
@@ -78,16 +77,19 @@ namespace location
     yg::Color m_locationAreaColor;
     yg::Color m_locationBorderColor;
 
-    math::Matrix<double, 3, 3> m_locationDrawM;
+    /// @todo Useless?
+    //math::Matrix<double, 3, 3> m_locationDrawM;
 
     yg::Color m_compassAreaColor;
     yg::Color m_compassBorderColor;
 
-    math::Matrix<double, 3, 3> m_compassDrawM;
+    /// @todo Useless?
+    //math::Matrix<double, 3, 3> m_compassDrawM;
 
     Framework * m_framework;
 
-    double m_cacheRadius;
+    /// @todo Useless?
+    //double m_cacheRadius;
 
     /// Compass Rendering Parameters
     /// @{