kwnevarez/google-maps-services-java

Java client library for Google Maps API Web Services

google-maps-services-java's Issues

make these fields private

Please make these fields private

rename the jsonPayload param to something more generic

Technically a POST payload, particularly of type String, could be non-JSON. Can you rename the jsonPayload param to something more generic, e.g. payload?

remove extra blank lines

Nit: remove extra blank lines

pass payload in as string

lower epsilon for lat/lng values

Generally, you probably want a lower epsilon for lat/lng values, 0.01 change can be quite large for these. That said, I think having such a high (low?) value for accuracy may make 0.01 about right if the results can move around.

Please include the copyright notice at the start of each new file

Please include the copyright notice at the start of each new file

make the post interface generic

Using GeolocationPostPayload makes this method highly specific to that API and thus not reusable. Please make this generic.

add validator to field with builder pattern

When you move across to the builder style, can you ensure you add a validator for this field?

confirm this api does not support client id

domesticmouse - can you confirm that this does not support client ID? I'm pretty sure it's right but would like an expert opinion :)

remove comment

Given that we know the user is using this Java API, please remove comments that are irrelevant, such as 'must be formatted as JSON'. Please do it across all files.

blank lines between methods

Nit: blank line between methods

test issue

test

add tower test case

Please don't check in commented-out code. I'd say to remove this but I think it'd be good to test that cell towers work as expected too, can you add it back in (either here or in another test)?

make this class extend from KeyOnlyAuthenticatedTest

As the API is marked as not supporting client IDs for authentication (in ApiConfig.supportsClientId(..) above), you will need to make this class extend from KeyOnlyAuthenticatedTest, otherwise it will attempt to run these tests using a client ID, where provided (i.e. in our automated tests).

set up a gson type handler that can serialise to ApiError

I've only had a quick look but it looks like (at least for the errors I've induced) that this API doesn't use the structure defined in ApiError. You'll need to either set up a gson type handler that can serialise to ApiError, or handle the fields individually here.

remove two blank lines

Nit: remove these two blank lines

use apiconfig to control GET vs POST

fix bad import

I think this is just a bad import from an IDE. You can drop the com.google.appengine.repackaged. prefix and you should be fine.

is it ok that the api will still be sent the default value for int

Are the int fields optional? If so, is it OK that the API will still be sent the default value for int even if the user doesn't provide it?

remove personal notes

Please remove your personal notes from this class.

remove log line

Please remove this log line.

builder-ize direction results, cell tower and wifi access point

https://github.com/googlemaps/google-maps-services-java/pull/164/files#r67978726

Rather than forcing developers to create a POJO and set each of the fields themselves, this library prefers to structure the API call method in a builder fashion.

e.g. in the Directions API we do this:

DirectionsResult result = DirectionsApi.newRequest(context)
    .mode(TravelMode.BICYCLING)
    .avoid(RouteRestriction.HIGHWAYS, RouteRestriction.TOLLS, RouteRestriction.FERRIES)
    .units(Unit.METRIC)
    .region("au")
    .origin("Sydney")
    .destination("Melbourne").await();

This saves users from having to create a separate object, allows for brevity for simple requests, and enables us to create an immutable payload (important for thread safety).

Can you please update this to follow the library's existing style (including for the cell tower and wifi classes)? You'll likely need to introduce a GeolocationApiRequest object, but you should be able to model it on the similar classes that exist.

make variables private

please make private, and move it up with the other private static final members.

move error mapping to "from" method

anchor javadoc above fields

Can you anchor the javadoc about each of these fields on the actual field, rather than in the class? (in all files please)

remove post from name

"Post" is an implementation detail, can you rename to simply GeolocationPayload?

use log to Level.CONFIG

Can you make this log to Level.CONFIG please?

confirm optional fields can take defaults

As these fields are optional, is it OK to send the default value for 'int' or 'boolean' if the user doesn't provide them?