Merge pull request #95 from google/cpp-recovery

Fix recovery bug in cpp

Author: drinckes

cpp/BUILD

@@ -26,19 +26,12 @@ cc_library(
     visibility = ["//visibility:private"],
 )
 
-filegroup(
-    name = "testdata",
-    srcs = glob([
-        "test_data/*",
-    ]),
-)
-
 cc_test(
     name = "openlocationcode_test",
     size = "small",
     srcs = ["openlocationcode_test.cc"],
     data = [
-        "testdata",
+        "//test_data:test_data",
     ],
     deps = [
         "@gtest//:main",
@@ -56,4 +49,4 @@ cc_binary(
     deps = [
         ":openlocationcode",
     ],
-)
+)

cpp/README.md

@@ -20,8 +20,8 @@ bazel build openlocationcode
 To run the tests, use:
 
 ```
-bazel test openlocationcode_test
+bazel test --test_output=all openlocationcode_test
 ```
 
 The tests use the CSV files in the test_data folder. Make sure you copy this folder to the
-root of your local workspace.
+root of your local workspace.

cpp/openlocationcode.cc

@@ -316,30 +316,31 @@ std::string RecoverNearest(const std::string &short_code,
   double resolution = pow_neg(
       internal::kEncodingBase, 2.0 - (padding_length / 2.0));
   // Distance from the center to an edge (in degrees).
-  double area_to_edge = resolution / 2.0;
+  double half_res = resolution / 2.0;
   // Use the reference location to pad the supplied short code and decode it.
   LatLng latlng = {latitude, longitude};
   std::string padding_code = Encode(latlng);
   CodeArea code_rect =
       Decode(std::string(padding_code.substr(0, padding_length)) +
              std::string(short_code));
   // How many degrees latitude is the code from the reference? If it is more
-  // than half the resolution, we need to move it east or west.
+  // than half the resolution, we need to move it north or south but keep it
+  // within -90 to 90 degrees.
   double center_lat = code_rect.GetCenter().latitude;
   double center_lng = code_rect.GetCenter().longitude;
-  if (center_lat - latitude > area_to_edge) {
-    // If the center of the short code is more than half a cell east,
-    // then the best match will be one position west.
+  if (latitude + half_res < center_lat && center_lat - resolution > internal::kLatitudeMaxDegrees) {
+    // If the proposed code is more than half a cell north of the reference location,
+    // it's too far, and the best match will be one cell south.
     center_lat -= resolution;
-  } else if (center_lat - latitude < -area_to_edge) {
-    // If the center of the short code is more than half a cell west,
-    // then the best match will be one position east.
+  } else if (latitude - half_res > center_lat && center_lat + resolution < internal::kLatitudeMaxDegrees) {
+    // If the proposed code is more than half a cell south of the reference location,
+    // it's too far, and the best match will be one cell north.
     center_lat += resolution;
   }
   // How many degrees longitude is the code from the reference?
-  if (center_lng - longitude > area_to_edge) {
+  if (longitude + half_res < center_lng) {
     center_lng -= resolution;
-  } else if (center_lng - longitude < -area_to_edge) {
+  } else if (longitude - half_res > center_lng) {
     center_lng += resolution;
   }
   LatLng center_latlng = {center_lat, center_lng};
@@ -472,4 +473,4 @@ size_t CodeLength(const std::string &code) {
   return clean_code.size();
 }
 
-}  // namespace openlocationcode
+}  // namespace openlocationcode

cpp/openlocationcode_test.cc

@@ -151,6 +151,7 @@ struct ShortCodeTestData {
   double reference_lat;
   double reference_lng;
   std::string short_code;
+  std::string test_type;
 };
 
 class ShortCodeChecks : public ::testing::TestWithParam<ShortCodeTestData> {};
@@ -167,6 +168,7 @@ std::vector<ShortCodeTestData> GetShortCodeDataFromCsv() {
     test_data.reference_lat = atof(csv_records[i][1].c_str());
     test_data.reference_lng = atof(csv_records[i][2].c_str());
     test_data.short_code = csv_records[i][3];
+    test_data.test_type = csv_records[i][4];
     data_results.push_back(test_data);
   }
   return data_results;
@@ -177,11 +179,15 @@ TEST_P(ShortCodeChecks, ShortCode) {
   LatLng reference_loc =
       LatLng{test_data.reference_lat, test_data.reference_lng};
   // Shorten the code using the reference location and check.
-  std::string actual_short = Shorten(test_data.full_code, reference_loc);
-  EXPECT_EQ(test_data.short_code, actual_short);
+  if (test_data.test_type == "B" || test_data.test_type == "S") {
+    std::string actual_short = Shorten(test_data.full_code, reference_loc);
+    EXPECT_EQ(test_data.short_code, actual_short);
+  }
   // Now extend the code using the reference location and check.
-  std::string actual_full = RecoverNearest(test_data.short_code, reference_loc);
-  EXPECT_EQ(test_data.full_code, actual_full);
+  if (test_data.test_type == "B" || test_data.test_type == "R") {
+    std::string actual_full = RecoverNearest(test_data.short_code, reference_loc);
+    EXPECT_EQ(test_data.full_code, actual_full);
+  }
 }
 
 INSTANTIATE_TEST_CASE_P(OLC_Tests, ShortCodeChecks,
@@ -206,4 +212,4 @@ TEST(MaxCodeLengthChecks, MaxCodeLength) {
 }
 
 }  // namespace
-}  // namespace openlocationcode
+}  // namespace openlocationcode