From dbdbe7adead4ee9f7db3a33f37ecbc22c72fe401 Mon Sep 17 00:00:00 2001 From: Aman Harishkumar Desai Date: Mon, 27 Nov 2023 19:44:56 -0400 Subject: [PATCH] fix three design smells and reformatted code --- .../google/maps/DistanceMatrixApiRequest.java | 10 +- .../com/google/maps/GeocodingApiRequest.java | 62 +++++----- .../java/com/google/maps/ImageResult.java | 2 + .../AndroidAuthenticationConfigProvider.java | 4 +- .../java/com/google/maps/android/Context.java | 4 +- .../com/google/maps/android/PackageInfo.java | 4 +- .../google/maps/internal/SafeEnumAdapter.java | 4 +- .../com/google/maps/internal/StringJoin.java | 4 +- .../internal/ratelimiter/RateLimiter.java | 1 + .../ratelimiter/SmoothRateLimiter.java | 4 + .../google/maps/model/EncodedPolyline.java | 4 +- .../google/maps/model/GeolocationPayload.java | 8 ++ .../com/google/maps/model/PlaceDetails.java | 59 +++++---- .../com/google/maps/model/SpeedLimit.java | 4 +- .../com/google/maps/GeolocationApiTest.java | 112 +++++++++--------- .../com/google/maps/StaticMapsApiTest.java | 1 + 16 files changed, 169 insertions(+), 118 deletions(-) diff --git a/src/main/java/com/google/maps/DistanceMatrixApiRequest.java b/src/main/java/com/google/maps/DistanceMatrixApiRequest.java index 12c7ee49b..a0f9a1993 100644 --- a/src/main/java/com/google/maps/DistanceMatrixApiRequest.java +++ b/src/main/java/com/google/maps/DistanceMatrixApiRequest.java @@ -21,6 +21,7 @@ import com.google.maps.DistanceMatrixApi.Response; import com.google.maps.model.*; import java.time.Instant; +import java.util.Arrays; /** A request to the Distance Matrix API. */ public class DistanceMatrixApiRequest @@ -96,10 +97,11 @@ public DistanceMatrixApiRequest destinations(LatLng... points) { * @return Returns this {@code DistanceMatrixApiRequest} for call chaining. */ public DistanceMatrixApiRequest mode(TravelMode mode) { - if (TravelMode.DRIVING.equals(mode) - || TravelMode.WALKING.equals(mode) - || TravelMode.BICYCLING.equals(mode) - || TravelMode.TRANSIT.equals(mode)) { + TravelMode[] validModes = { + TravelMode.DRIVING, TravelMode.WALKING, TravelMode.BICYCLING, TravelMode.TRANSIT + }; + + if (Arrays.asList(validModes).contains(mode)) { return param("mode", mode); } throw new IllegalArgumentException( diff --git a/src/main/java/com/google/maps/GeocodingApiRequest.java b/src/main/java/com/google/maps/GeocodingApiRequest.java index 0591953bd..ecfd2b43a 100644 --- a/src/main/java/com/google/maps/GeocodingApiRequest.java +++ b/src/main/java/com/google/maps/GeocodingApiRequest.java @@ -33,21 +33,33 @@ public GeocodingApiRequest(GeoApiContext context) { @Override protected void validateRequest() { // Must not have both address and latlng. - if (params().containsKey("latlng") - && params().containsKey("address") - && params().containsKey("place_id")) { - throw new IllegalArgumentException( - "Request must contain only one of 'address', 'latlng' or 'place_id'."); - } + validateRequestHasOnlyOne("address", "latlng", "place_id"); // Must contain at least one of place_id, address, latlng, and components; - if (!params().containsKey("latlng") - && !params().containsKey("address") - && !params().containsKey("components") - && !params().containsKey("place_id")) { - throw new IllegalArgumentException( - "Request must contain at least one of 'address', 'latlng', 'place_id' and 'components'."); + validateRequestHasAtLeastOne("address", "latlng", "place_id", "components"); + } + + private void validateRequestHasOnlyOne(String... keys) { + for (String key : keys) { + if (params().containsKey(key)) { + for (String otherKey : keys) { + if (!key.equals(otherKey) && params().containsKey(otherKey)) { + throw new IllegalArgumentException( + "Request must not contain both '" + key + "' and '" + otherKey + "'."); + } + } + } + } + } + + private void validateRequestHasAtLeastOne(String... keys) { + for (String key : keys) { + if (params().containsKey(key)) { + return; + } } + throw new IllegalArgumentException( + "Request must contain at least one of '" + join("', '", keys) + "'."); } /** @@ -60,6 +72,17 @@ public GeocodingApiRequest address(String address) { return param("address", address); } + /** + * Custom parameter. For advanced usage only. + * + * @param parameter The name of the custom parameter. + * @param value The value of the custom parameter. + * @return Returns the request for call chaining. + */ + public GeocodingApiRequest custom(String parameter, String value) { + return param(parameter, value); + } + /** * Creates a forward geocode for {@code placeId}. * @@ -111,21 +134,6 @@ public GeocodingApiRequest region(String region) { return param("region", region); } - /** - * Sets the component filters. Each component filter consists of a component:value pair and will - * fully restrict the results from the geocoder. - * - *

For more information see - * Component Filtering. - * - * @param filters Component filters to apply to the request. - * @return Returns this {@code GeocodingApiRequest} for call chaining. - */ - public GeocodingApiRequest components(ComponentFilter... filters) { - return param("components", join('|', filters)); - } - /** * Sets the result type. Specifying a type will restrict the results to this type. If multiple * types are specified, the API will return all addresses that match any of the types. diff --git a/src/main/java/com/google/maps/ImageResult.java b/src/main/java/com/google/maps/ImageResult.java index 6c718648c..3d944f9aa 100644 --- a/src/main/java/com/google/maps/ImageResult.java +++ b/src/main/java/com/google/maps/ImageResult.java @@ -23,8 +23,10 @@ public class ImageResult implements Serializable { private static final long serialVersionUID = 1L; + /** The image data from the Photos API call. */ public final byte[] imageData; + /** The Content-Type header of the returned result. */ public final String contentType; diff --git a/src/main/java/com/google/maps/android/AndroidAuthenticationConfigProvider.java b/src/main/java/com/google/maps/android/AndroidAuthenticationConfigProvider.java index 861fad33c..86b724798 100644 --- a/src/main/java/com/google/maps/android/AndroidAuthenticationConfigProvider.java +++ b/src/main/java/com/google/maps/android/AndroidAuthenticationConfigProvider.java @@ -6,7 +6,9 @@ */ public class AndroidAuthenticationConfigProvider { - /** @return the environment specific {@link AndroidAuthenticationConfig} */ + /** + * @return the environment specific {@link AndroidAuthenticationConfig} + */ public AndroidAuthenticationConfig provide() { Context context = Context.getApplicationContext(); if (context == null) { diff --git a/src/main/java/com/google/maps/android/Context.java b/src/main/java/com/google/maps/android/Context.java index d23981a1f..6bae427f4 100644 --- a/src/main/java/com/google/maps/android/Context.java +++ b/src/main/java/com/google/maps/android/Context.java @@ -32,7 +32,9 @@ public static Context getApplicationContext() { } } - /** @return the package name of the Android app if available, otherwise, null */ + /** + * @return the package name of the Android app if available, otherwise, null + */ @Nullable public String getPackageName() { try { diff --git a/src/main/java/com/google/maps/android/PackageInfo.java b/src/main/java/com/google/maps/android/PackageInfo.java index 4ca9467c6..ab66dc14c 100644 --- a/src/main/java/com/google/maps/android/PackageInfo.java +++ b/src/main/java/com/google/maps/android/PackageInfo.java @@ -13,7 +13,9 @@ public PackageInfo(Class piClass, Object piInstance) { this.piInstance = piInstance; } - /** @return the signing signature for the app */ + /** + * @return the signing signature for the app + */ @Nullable public Object signingSignature() { try { diff --git a/src/main/java/com/google/maps/internal/SafeEnumAdapter.java b/src/main/java/com/google/maps/internal/SafeEnumAdapter.java index 19ab6848c..856abad76 100644 --- a/src/main/java/com/google/maps/internal/SafeEnumAdapter.java +++ b/src/main/java/com/google/maps/internal/SafeEnumAdapter.java @@ -38,7 +38,9 @@ public class SafeEnumAdapter> extends TypeAdapter { private final Class clazz; private final E unknownValue; - /** @param unknownValue the value to return if the value cannot be found. */ + /** + * @param unknownValue the value to return if the value cannot be found. + */ public SafeEnumAdapter(E unknownValue) { if (unknownValue == null) throw new IllegalArgumentException(); diff --git a/src/main/java/com/google/maps/internal/StringJoin.java b/src/main/java/com/google/maps/internal/StringJoin.java index f52b0dd7d..84067783d 100644 --- a/src/main/java/com/google/maps/internal/StringJoin.java +++ b/src/main/java/com/google/maps/internal/StringJoin.java @@ -67,7 +67,9 @@ public static String join(char delim, UrlValue... parts) { * string joinable. */ public interface UrlValue { - /** @return the object, represented as a URL value (not URL encoded). */ + /** + * @return the object, represented as a URL value (not URL encoded). + */ String toUrlValue(); } } diff --git a/src/main/java/com/google/maps/internal/ratelimiter/RateLimiter.java b/src/main/java/com/google/maps/internal/ratelimiter/RateLimiter.java index 4ba8860d2..04826c4fb 100644 --- a/src/main/java/com/google/maps/internal/ratelimiter/RateLimiter.java +++ b/src/main/java/com/google/maps/internal/ratelimiter/RateLimiter.java @@ -100,6 +100,7 @@ public abstract class RateLimiter { * object to facilitate testing. */ private final SleepingStopwatch stopwatch; + // Can't be initialized in the constructor because mocks don't call the constructor. private volatile Object mutexDoNotUseDirectly; diff --git a/src/main/java/com/google/maps/internal/ratelimiter/SmoothRateLimiter.java b/src/main/java/com/google/maps/internal/ratelimiter/SmoothRateLimiter.java index 67c7f228d..2451163bd 100644 --- a/src/main/java/com/google/maps/internal/ratelimiter/SmoothRateLimiter.java +++ b/src/main/java/com/google/maps/internal/ratelimiter/SmoothRateLimiter.java @@ -151,13 +151,16 @@ abstract class SmoothRateLimiter extends RateLimiter { /** The currently stored permits. */ double storedPermits; + /** The maximum number of stored permits. */ double maxPermits; + /** * The interval between two unit requests, at our stable rate. E.g., a stable rate of 5 permits * per second has a stable interval of 200ms. */ double stableIntervalMicros; + /** * The time when the next request (no matter its size) will be granted. After granting a request, * this is pushed further in the future. Large requests push this further than small requests. @@ -292,6 +295,7 @@ void resync(long nowMicros) { */ static final class SmoothWarmingUp extends SmoothRateLimiter { private final long warmupPeriodMicros; + /** * The slope of the line from the stable interval (when permits == 0), to the cold interval * (when permits == maxPermits) diff --git a/src/main/java/com/google/maps/model/EncodedPolyline.java b/src/main/java/com/google/maps/model/EncodedPolyline.java index 07f75ddeb..f4dee9c02 100644 --- a/src/main/java/com/google/maps/model/EncodedPolyline.java +++ b/src/main/java/com/google/maps/model/EncodedPolyline.java @@ -42,7 +42,9 @@ public EncodedPolyline(String encodedPoints) { this.points = encodedPoints; } - /** @param points A path as a collection of {@code LatLng} points. */ + /** + * @param points A path as a collection of {@code LatLng} points. + */ public EncodedPolyline(List points) { this.points = PolylineEncoding.encode(points); } diff --git a/src/main/java/com/google/maps/model/GeolocationPayload.java b/src/main/java/com/google/maps/model/GeolocationPayload.java index 4d33da9ba..d29bff445 100644 --- a/src/main/java/com/google/maps/model/GeolocationPayload.java +++ b/src/main/java/com/google/maps/model/GeolocationPayload.java @@ -36,30 +36,38 @@ public class GeolocationPayload implements Serializable { private static final long serialVersionUID = 1L; + /** The mobile country code (MCC) for the device's home network. */ public Integer homeMobileCountryCode = null; + /** The mobile network code (MNC) for the device's home network. */ public Integer homeMobileNetworkCode = null; + /** * The mobile radio type. Supported values are {@code "lte"}, {@code "gsm"}, {@code "cdma"}, and * {@code "wcdma"}. While this field is optional, it should be included if a value is available, * for more accurate results. */ public String radioType = null; + /** The carrier name. */ public String carrier = null; + /** * Specifies whether to fall back to IP geolocation if wifi and cell tower signals are not * available. Note that the IP address in the request header may not be the IP of the device. * Defaults to true. Set considerIp to false to disable fall back. */ public Boolean considerIp = null; + /** An array of cell tower objects. See {@link com.google.maps.model.CellTower}. */ public CellTower[] cellTowers; + /** An array of WiFi access point objects. See {@link com.google.maps.model.WifiAccessPoint}. */ public WifiAccessPoint[] wifiAccessPoints; public GeolocationPayload() {} + // constructor only used by the builder class below private GeolocationPayload( Integer _homeMobileCountryCode, diff --git a/src/main/java/com/google/maps/model/PlaceDetails.java b/src/main/java/com/google/maps/model/PlaceDetails.java index af0d2023e..b2f999454 100644 --- a/src/main/java/com/google/maps/model/PlaceDetails.java +++ b/src/main/java/com/google/maps/model/PlaceDetails.java @@ -230,18 +230,8 @@ public String toString() { if (altIds != null && altIds.length > 0) { sb.append(", altIds=").append(Arrays.toString(altIds)); } - if (formattedPhoneNumber != null) { - sb.append(", phone=").append(formattedPhoneNumber); - } - if (internationalPhoneNumber != null) { - sb.append(", internationalPhoneNumber=").append(internationalPhoneNumber); - } - if (url != null) { - sb.append(", url=").append(url); - } - if (website != null) { - sb.append(", website=").append(website); - } + sb.append(addPhoneNumber(sb)); + sb.append(addWebsiteInfo(sb)); if (icon != null) { sb.append(", icon"); } @@ -271,6 +261,41 @@ public String toString() { if (secondaryOpeningHours != null) { sb.append(", secondaryOpeningHours=").append(secondaryOpeningHours); } + sb.append(addServingInfo(sb)); + if (takeout != null) { + sb.append(", takeout=").append(takeout); + } + if (wheelchairAccessibleEntrance != null) { + sb.append(", wheelchairAccessibleEntrance=").append(wheelchairAccessibleEntrance); + } + if (htmlAttributions != null && htmlAttributions.length > 0) { + sb.append(", ").append(htmlAttributions.length).append(" htmlAttributions"); + } + sb.append("]"); + return sb.toString(); + } + + private String addPhoneNumber(StringBuilder sb) { + if (formattedPhoneNumber != null) { + sb.append(", phone=").append(formattedPhoneNumber); + } + if (internationalPhoneNumber != null) { + sb.append(", internationalPhoneNumber=").append(internationalPhoneNumber); + } + return sb.toString(); + } + + private String addWebsiteInfo(StringBuilder sb) { + if (url != null) { + sb.append(", url=").append(url); + } + if (website != null) { + sb.append(", website=").append(website); + } + return sb.toString(); + } + + private String addServingInfo(StringBuilder sb) { if (servesBeer != null) { sb.append(", servesBeer=").append(servesBeer); } @@ -292,16 +317,6 @@ public String toString() { if (servesWine != null) { sb.append(", servesWine=").append(servesWine); } - if (takeout != null) { - sb.append(", takeout=").append(takeout); - } - if (wheelchairAccessibleEntrance != null) { - sb.append(", wheelchairAccessibleEntrance=").append(wheelchairAccessibleEntrance); - } - if (htmlAttributions != null && htmlAttributions.length > 0) { - sb.append(", ").append(htmlAttributions.length).append(" htmlAttributions"); - } - sb.append("]"); return sb.toString(); } diff --git a/src/main/java/com/google/maps/model/SpeedLimit.java b/src/main/java/com/google/maps/model/SpeedLimit.java index 313e27079..cdbbb35aa 100644 --- a/src/main/java/com/google/maps/model/SpeedLimit.java +++ b/src/main/java/com/google/maps/model/SpeedLimit.java @@ -35,7 +35,9 @@ public class SpeedLimit implements Serializable { */ public double speedLimit; - /** @return Returns the speed limit in miles per hour (MPH). */ + /** + * @return Returns the speed limit in miles per hour (MPH). + */ public long speedLimitMph() { return Math.round(speedLimit * 0.621371); } diff --git a/src/test/java/com/google/maps/GeolocationApiTest.java b/src/test/java/com/google/maps/GeolocationApiTest.java index 8388e17c0..53738eb11 100644 --- a/src/test/java/com/google/maps/GeolocationApiTest.java +++ b/src/test/java/com/google/maps/GeolocationApiTest.java @@ -64,27 +64,27 @@ public void testDocSampleGeolocation() throws Exception { .AddCellTower( new CellTower.CellTowerBuilder() .CellId(39627456) - .LocationAreaCode(40495) - .MobileCountryCode(310) - .MobileNetworkCode(260) - .Age(0) - .SignalStrength(-95) - .createCellTower()) + .LocationAreaCode(40495) + .MobileCountryCode(310) + .MobileNetworkCode(260) + .Age(0) + .SignalStrength(-95) + .createCellTower()) .AddWifiAccessPoint( new WifiAccessPoint.WifiAccessPointBuilder() .MacAddress("01:23:45:67:89:AB") - .SignalStrength(-65) - .SignalToNoiseRatio(8) - .Channel(8) - .Age(0) - .createWifiAccessPoint()) + .SignalStrength(-65) + .SignalToNoiseRatio(8) + .Channel(8) + .Age(0) + .createWifiAccessPoint()) .AddWifiAccessPoint( new WifiAccessPoint.WifiAccessPointBuilder() .MacAddress("01:23:45:67:89:AC") - .SignalStrength(4) - .SignalToNoiseRatio(4) - .Age(0) - .createWifiAccessPoint()) + .SignalStrength(4) + .SignalToNoiseRatio(4) + .Age(0) + .createWifiAccessPoint()) .CreatePayload() .await(); @@ -110,12 +110,10 @@ public void testMinimumWifiGeolocation() throws Exception { .ConsiderIp(false) .AddWifiAccessPoint( new WifiAccessPoint.WifiAccessPointBuilder() - .MacAddress("94:b4:0f:ff:6b:11") - .createWifiAccessPoint()) + .MacAddress("94:b4:0f:ff:6b:11").createWifiAccessPoint()) .AddWifiAccessPoint( new WifiAccessPoint.WifiAccessPointBuilder() - .MacAddress("94:b4:0f:ff:6b:10") - .createWifiAccessPoint()) + .MacAddress("94:b4:0f:ff:6b:10").createWifiAccessPoint()) .CreatePayload() .await(); @@ -141,15 +139,15 @@ public void testBasicGeolocation() throws Exception { .AddWifiAccessPoint( new WifiAccessPoint.WifiAccessPointBuilder() .MacAddress("92:68:c3:f8:76:47") - .SignalStrength(-42) - .SignalToNoiseRatio(68) - .createWifiAccessPoint()) + .SignalStrength(-42) + .SignalToNoiseRatio(68) + .createWifiAccessPoint()) .AddWifiAccessPoint( new WifiAccessPoint.WifiAccessPointBuilder() .MacAddress("94:b4:0f:ff:6b:11") - .SignalStrength(-55) - .SignalToNoiseRatio(55) - .createWifiAccessPoint()) + .SignalStrength(-55) + .SignalToNoiseRatio(55) + .createWifiAccessPoint()) .CreatePayload() .await(); @@ -178,12 +176,10 @@ public void testAlternateWifiSetterGeolocation() throws Exception { WifiAccessPoint[] wifiAccessPoints = new WifiAccessPoint[2]; wifiAccessPoints[0] = new WifiAccessPoint.WifiAccessPointBuilder() - .MacAddress("94:b4:0f:ff:6b:11") - .createWifiAccessPoint(); + .MacAddress("94:b4:0f:ff:6b:11").createWifiAccessPoint(); wifiAccessPoints[1] = new WifiAccessPoint.WifiAccessPointBuilder() - .MacAddress("94:b4:0f:ff:6b:10") - .createWifiAccessPoint(); + .MacAddress("94:b4:0f:ff:6b:10").createWifiAccessPoint(); GeolocationResult result = GeolocationApi.newRequest(sc.context) @@ -220,19 +216,19 @@ public void testMaximumWifiGeolocation() throws Exception { .AddWifiAccessPoint( new WifiAccessPoint.WifiAccessPointBuilder() .MacAddress("94:b4:0f:ff:88:31") - .SignalStrength(-61) - .SignalToNoiseRatio(49) - .Channel(40) - .Age(0) - .createWifiAccessPoint()) + .SignalStrength(-61) + .SignalToNoiseRatio(49) + .Channel(40) + .Age(0) + .createWifiAccessPoint()) .AddWifiAccessPoint( new WifiAccessPoint.WifiAccessPointBuilder() .MacAddress("94:b4:0f:ff:88:30") - .SignalStrength(-64) - .SignalToNoiseRatio(46) - .Channel(40) - .Age(0) - .createWifiAccessPoint()) + .SignalStrength(-64) + .SignalToNoiseRatio(46) + .Channel(40) + .Age(0) + .createWifiAccessPoint()) .CreatePayload() .await(); @@ -272,10 +268,10 @@ public void testMinimumCellTowerGeolocation() throws Exception { .AddCellTower( new CellTower.CellTowerBuilder() .CellId(39627456) - .LocationAreaCode(40495) - .MobileCountryCode(310) - .MobileNetworkCode(260) - .createCellTower()) + .LocationAreaCode(40495) + .MobileCountryCode(310) + .MobileNetworkCode(260) + .createCellTower()) .CreatePayload() .await(); @@ -301,14 +297,14 @@ public void testAlternatePayloadBuilderGeolocation() throws Exception { GeolocationPayload payload = new GeolocationPayload.GeolocationPayloadBuilder() .ConsiderIp(false) - .AddCellTower( - new CellTower.CellTowerBuilder() - .CellId(39627456) - .LocationAreaCode(40495) - .MobileCountryCode(310) - .MobileNetworkCode(260) - .createCellTower()) - .createGeolocationPayload(); + .AddCellTower( + new CellTower.CellTowerBuilder() + .CellId(39627456) + .LocationAreaCode(40495) + .MobileCountryCode(310) + .MobileNetworkCode(260) + .createCellTower()) + .createGeolocationPayload(); GeolocationResult result = GeolocationApi.geolocate(sc.context, payload).await(); assertNotNull(result.toString()); @@ -338,13 +334,13 @@ public void testMaximumCellTowerGeolocation() throws Exception { .AddCellTower( new CellTower.CellTowerBuilder() .CellId(39627456) - .LocationAreaCode(40495) - .MobileCountryCode(310) - .MobileNetworkCode(260) - .Age(0) - .SignalStrength(-103) - .TimingAdvance(15) - .createCellTower()) + .LocationAreaCode(40495) + .MobileCountryCode(310) + .MobileNetworkCode(260) + .Age(0) + .SignalStrength(-103) + .TimingAdvance(15) + .createCellTower()) .CreatePayload() .await(); diff --git a/src/test/java/com/google/maps/StaticMapsApiTest.java b/src/test/java/com/google/maps/StaticMapsApiTest.java index bc00b3064..6cf54057f 100644 --- a/src/test/java/com/google/maps/StaticMapsApiTest.java +++ b/src/test/java/com/google/maps/StaticMapsApiTest.java @@ -40,6 +40,7 @@ public class StaticMapsApiTest { private final int HEIGHT = 480; private final LatLng MELBOURNE = new LatLng(-37.8136, 144.9630); private final LatLng SYDNEY = new LatLng(-33.8688, 151.2093); + /** This encoded path matches the exact [MELBOURNE, SYDNEY] points. */ private final String MELBOURNE_TO_SYDNEY_ENCODED_POLYLINE = "~mxeFwaxsZ_naWk~be@";