[search] Refactor MatchLatLonDegree function #3301

Open

trueTatar wants to merge 1 commit from trueTatar/latlon-refactoring into master

trueTatar commented

2022-08-30 16:43:59 +00:00

(Migrated from github.com)

Fixed the behavior of the function with the same symbols of the cardinal directions (the corresponding unit tests were added);
Added unit tests for edge cases for each coordinate format;
Adjusted some unit tests to discribe new behavior of the function more accurately.

The implemented version of the MatchLatLonDegree function makes it easy to add new or remove existing coordinate formats. The implementation strictly follows the generally accepted DD, DMS and DDM (fix #3121) notations (a description of the notations is given in the comments to the classes). It was also done: - Fixed the behavior of the function with the same symbols of the cardinal directions (the corresponding unit tests were added); - Added unit tests for edge cases for each coordinate format; - Adjusted some unit tests to discribe new behavior of the function more accurately.

👍 1

vng (Migrated from github.com) reviewed 2022-08-30 16:43:59 +00:00

biodranik commented

2022-09-02 20:37:32 +00:00

(Migrated from github.com)

Did you test it on iOS and Android?

Please also fix failed tests in checks, e.g.:

FAILED
search_integration_tests/postcode_points_tests.cpp:137 TEST(result.GetResultType() == Result::Type::Postcode) LatLon Postcode

Did you test it on iOS and Android? Please also fix failed tests in checks, e.g.: ``` FAILED search_integration_tests/postcode_points_tests.cpp:137 TEST(result.GetResultType() == Result::Type::Postcode) LatLon Postcode ```

biodranik (Migrated from github.com) reviewed 2022-09-12 00:51:34 +00:00

biodranik (Migrated from github.com) left a comment

Your current implementation can be improved:

It is not performance-optimal (unnecessary strlen, virtual inheritance, etc.). Each typed character passes it, so it is very important for OM and the device's battery.
The code looks more complex and verbose than it was before. Are patterns from books really needed here? Can it be faster and simpler?

For sure we'll add some more formats. But their parsing should be easy and straightforward. Ideally, in one pass.

Your current implementation can be improved: 1. It is not performance-optimal (unnecessary strlen, virtual inheritance, etc.). Each typed character passes it, so it is very important for OM and the device's battery. 2. The code looks more complex and verbose than it was before. Are patterns from books really needed here? Can it be _faster_ and _simpler_? For sure we'll add some more formats. But their parsing should be easy and straightforward. Ideally, in one pass.

This repo is archived. You cannot comment on pull requests.

No reviewers

vng