search

ericnewton76 / gmaps-api-net

C# google maps api interface for interacting with the backend web services for Google Maps
Apache License 2.0
284 stars 149 forks source link

Converting between camel-case enums and the Google API equivalents is a pain #84

Open richardthombs opened 7 years ago

richardthombs commented 7 years ago

Our API uses camel-case for enum values, ie AdministrativeAreaLevel1 whereas the Google Maps API uses lowercase with words separated by underscores: administrative_area_level_1.

This approach has created too much code that uses massive switch statements that convert strings to enums and back again. This makes it harder to keep the code up to date with the API, because adding and removing enum values means code has to be updated in several locations. It also leads to bugs like #74.

I suggest we either:

  1. Replace all our camel-case enums with ones that match the Google API so we can do a direct mapping using Enum.Parse and ToString. Obviously this would break existing code, although nothing that couldn't be fixed with minimal effort.

  2. We create an enum helper that can do the conversions for us. This would involve iterating through the characters of an enum name in order to find the uppercase characters and adding an underscore, so it is likely to incur a performance hit.

Personally I favour option 1 as it will be faster and simpler.

ericnewton76 commented 7 years ago

Doesnt the Newtonsoft Json parser library have an option similar to this? Like CamelCaseProperty or something? I'm going to look and update this comment after

EDIT: So after a bit, I'm leaning towards registering an enum parser with the json.net parsing that can do this.

Here's a stackoverflow fix for "snake case" (using underscores) https://stackoverflow.com/a/30456701/323456

Perhaps will incorporate that routine into our json response parser.

richardthombs commented 7 years ago

So long as the conversions are handled automatically, rather than relying on a human to correctly convert "place_of_worship" -> "PlaceOfWorship" and back again, I'm all for it.

I'm a big fan of optimise later :smile:

And this way we'll keep the API the same and after all, CamelCase is probably what most of our users expect.

As you might have guessed from the flood of pull requests, I'm between jobs at the moment, so I'll look into implementing this, if you like?

ericnewton76 commented 7 years ago

Between jobs: Lol, as I am, too...

richardthombs commented 7 years ago

Well maybe I can pay you to make this change?

If not, I just found this: http://www.newtonsoft.com/json/help/html/NamingStrategySnakeCase.htm

So maybe it's built into newer versions of Newtonsoft.Json.

ericnewton76 commented 7 years ago

Yes I believe its in the newer Jsonsoft's. I was going to try to tackle this.

UPDATE: Yes, SnakeCaseNamingStrategy is in Json.Net v10+. I'm slightly worried about moving so far forward since v8. I'm assuming any consumers of this library can easily move up to v10 by making this change...

richardthombs commented 7 years ago

If we wanted to avoid requiring v10 of Newtonsoft.Json, then I'm sure we can implement this as a custom converter in older versions of Newtonsoft.Json without too much trouble.

But I don't see that much a problem in requiring folks to upgrade to v10.