Speed conversion unification #2993

2022-07-19T14:18:21Z

AntonM030481 commented

2022-07-19 14:18:21 +00:00

(Migrated from github.com)

No description provided.

biodranik (Migrated from github.com) reviewed 2022-07-19 20:55:39 +00:00

android/jni/com/mapswithme/util/StringUtils.cpp

					
				@ -60,3 +58,4 @@

				  settings::Get(settings::kMeasurementUnits, units);

				  return MakeJavaPair(env, measurement_utils::FormatSpeedNumeric(metersPerSecond, units),

				                      measurement_utils::FormatSpeedUnits(units));

				}

biodranik (Migrated from github.com) commented

2022-07-19 20:44:48 +00:00

Надо будет отрефакторить Get(), чтобы принимало дефолтное значение.

drape_frontend/my_position_controller.cpp

biodranik (Migrated from github.com) commented

2022-07-19 20:48:19 +00:00

Эти массивы разве нельзя переписать как

static std::array<TSpeedScale, 6> constexpr scales3d = {{
  {20.0, 0.23},
  {40.0, 0.75},
  // ...
}};

Эти массивы разве нельзя переписать как ``` static std::array<TSpeedScale, 6> constexpr scales3d = {{ {20.0, 0.23}, {40.0, 0.75}, // ... }}; ```

drape_frontend/my_position_controller.cpp

					
				@ -67,0 +65,4 @@

				    {75.0, 2.50},

				    {85.0, 3.75},

				    {95.0, 6.00},

				  }};

biodranik (Migrated from github.com) commented

2022-07-19 20:45:26 +00:00

MetersPerSecond or MilesPerSecond?

platform/measurement_utils.cpp

biodranik (Migrated from github.com) commented

2022-07-19 20:53:52 +00:00

Лучше избежать ненужных промежуточных вычислений, и сразу считать в нужных единицах, особенно если метод будет дёргаться часто. Или компилятор всё сам соптимизирует?

routing/routing_tests/routing_session_test.cpp

					
				@ -337,3 +337,3 @@

				    info.m_latitude = 1.;

				    info.m_speedMpS = routing::KMPH2MPS(60);

				    info.m_speedMpS = measurement_utils::KmphToMps(60);

				    SessionState code = SessionState::NoValidRoute;

biodranik (Migrated from github.com) commented

2022-07-19 20:51:37 +00:00

Можно внутри неймспейса сделать using measurement_utils::KmphToMps;

AntonM030481 (Migrated from github.com) reviewed 2022-07-19 20:58:30 +00:00

platform/measurement_utils.cpp

AntonM030481 (Migrated from github.com) commented

2022-07-19 20:58:30 +00:00

Уверен что оптимизирует. 2 inline функции с умножением на константу внутри.

AntonM030481 (Migrated from github.com) reviewed 2022-07-19 21:02:14 +00:00

drape_frontend/my_position_controller.cpp

					
				@ -67,0 +65,4 @@

				    {75.0, 2.50},

				    {85.0, 3.75},

				    {95.0, 6.00},

				  }};

AntonM030481 (Migrated from github.com) commented

2022-07-19 21:02:14 +00:00

У нас не используется Мили в секунду. Только мили в час.
В данном случае это метры в секунду, и для них наш стандартный постфикс MpS.

У нас не используется Мили в секунду. Только мили в час. В данном случае это метры в секунду, и для них наш стандартный постфикс MpS.

AntonM030481 (Migrated from github.com) reviewed 2022-07-19 21:03:54 +00:00

android/jni/com/mapswithme/util/StringUtils.cpp

					
				@ -60,3 +58,4 @@

				  settings::Get(settings::kMeasurementUnits, units);

				  return MakeJavaPair(env, measurement_utils::FormatSpeedNumeric(metersPerSecond, units),

				                      measurement_utils::FormatSpeedUnits(units));

				}

AntonM030481 (Migrated from github.com) commented

2022-07-19 21:03:54 +00:00

Пока оставим так.

biodranik (Migrated from github.com) reviewed 2022-07-20 05:53:22 +00:00

drape_frontend/my_position_controller.cpp

					
				@ -67,0 +65,4 @@

				    {75.0, 2.50},

				    {85.0, 3.75},

				    {95.0, 6.00},

				  }};

biodranik (Migrated from github.com) commented

2022-07-20 05:53:22 +00:00

Имхо, лучше избегать неоднозначности, и называть длиннее.

vng (Migrated from github.com) approved these changes 2022-07-20 06:55:50 +00:00

AntonM030481 (Migrated from github.com) reviewed 2022-07-20 07:06:07 +00:00

drape_frontend/my_position_controller.cpp

					
				@ -67,0 +65,4 @@

				    {75.0, 2.50},

				    {85.0, 3.75},

				    {95.0, 6.00},

				  }};

AntonM030481 (Migrated from github.com) commented

2022-07-20 07:06:07 +00:00

С локальными переменными проще.

Я дольше боюсь за глобальные функции
MphToKmph(double mph)
KmphToMph(double kmph)
KmphToMps(double kmph)
Две последние легко перепутать.
Можно договориться для миль использовать mi везде (в соответствии с общепринятым обозначением).
Так уже сделано в MilesToMeters(double mi).
Тогда для скорости получится miph, speedMipH, KmphToMiph
Поскольку конверсии в мили очень редки, я бы согласился
на чуть корявые названия, зато сильно отличающиеся от популярных.

С локальными переменными проще. Я дольше боюсь за глобальные функции MphToKmph(double mph) KmphToMph(double kmph) KmphToMps(double kmph) Две последние легко перепутать. Можно договориться для миль использовать mi везде (в соответствии с общепринятым обозначением). Так уже сделано в MilesToMeters(double mi). Тогда для скорости получится miph, speedMipH, KmphToMiph Поскольку конверсии в мили очень редки, я бы согласился на чуть корявые названия, зато сильно отличающиеся от популярных.

biodranik (Migrated from github.com) reviewed 2022-07-20 07:36:01 +00:00

drape_frontend/my_position_controller.cpp

					
				@ -67,0 +65,4 @@

				    {75.0, 2.50},

				    {85.0, 3.75},

				    {95.0, 6.00},

				  }};

biodranik (Migrated from github.com) commented

2022-07-20 07:36:01 +00:00

Я за однозначную, пускай и корявость )

AntonM030481 (Migrated from github.com) reviewed 2022-07-20 07:42:47 +00:00

drape_frontend/my_position_controller.cpp

AntonM030481 (Migrated from github.com) commented

2022-07-20 07:42:46 +00:00

OK

AntonM030481 (Migrated from github.com) reviewed 2022-07-20 07:44:55 +00:00

routing/routing_tests/routing_session_test.cpp

					
				@ -337,3 +337,3 @@

				    info.m_latitude = 1.;

				    info.m_speedMpS = routing::KMPH2MPS(60);

				    info.m_speedMpS = measurement_utils::KmphToMps(60);

				    SessionState code = SessionState::NoValidRoute;