gabzim / circle-to-polygon

Receives a Coordinate, a Radius and a Number of edges and aproximates a circle by creating a polygon that fills its area
ISC License
113 stars 29 forks source link

Issue 12 Allow `center` to be an Object #15

Closed johachi closed 4 years ago

johachi commented 4 years ago

This PR extends circleToPolygon so the function accepts center coordinate either as an array or as an object. Behavior is unchanged when the center coordinate is an array.

Key-pair priorization

Passing the lat and lon values in an object causes the complication that the object can contain more than two keys, and some of the keys might seem to be duplicates (eg: { lat, latitude, longitude }, where lat does not mean "latitude").

This problem can be solved in several ways. I chose to solve it by looking for complete key-pairs. The first found complete key-par is being used (no matter if the keys has the correct values or not). Key pairs are being looked for in the following order:

  1. { lat: latValue, lon: lonValue }
  2. { lat: latValue, lng: lonValue }
  3. { latitude: latValue, longitude: lonValue }

In other words;

Error handling

No valid key pairs

circleToPolygon will throw an error if no complete key pairs are found (when center argument is an object).

In other words, the following key pairs will throw an error:

No valid values

An error will be thrown if the first found complete key pair does not have the correct type or values. For example: { lat: "General" lon: "Kenobi", latitude: 120.827492, longitude: 40.729362 } will throw an error since lat/lon is used instead of latitude/longitude

Other

I have chose to accept objects such as { lat, latitude, longitude } since maybe it makes sense for someone to have the key lat in the center object for use somewhere else, idk...

I'm wondering if it makes more sense to prioritize the keys pair latitude/longitude the highest, instead of lowest...

Also, should I have it look for the first valid key-pair instead of the first complete key pair?

I'm open to changing this behavior is requested.

This closes #12 .

johachi commented 4 years ago

I'm closing this PR since it's quite big and contain some things that is not connected to the issue. I will instead break the PR into several smaller PR's that are much more easy to check. Easier to check several small PR's with small changed than one big PR.