Changing GVRS API, need comments
gwlucastrig opened this issue · 1 comments
As I was writing the "getting started" pages for the Gridfour wiki, I realized that I'd made a mistake in the way I created some of the coordinate-transform methods in the GVRS API.
As a solution, I am preparing for release 1.0.1 in the near future.
I'd like to get some user comments on this issue. In particular, I'd like to get comments on my proposed approach to making this change. I would prefer to remove the old methods and replace them with the new. Ordinarily, my approach to this issue would be to deprecate the old methods and remove them in the future. But since GVRS is so new, and so few users have adopted it (so far), I think it would be cleaner to just make the change. So I would like to get some user feedback on how much of an inconvenience this would be.
The Proposed Change
The existing code includes methods in the form:
double []gridPoint = spec.mapGeographicToGrid(latitude, longitude);
double []geoPoint = spec.mapGridToGeographic(gridPoint[0], gridPoint[1];
I think that the use of arrays was a mistake. First off, they are error prone. It's easy to get confused as
to which array index refers to which coordinate. Is geoPoint[0] a latitude or a longitude? Unless you know the API pretty well, you may be unsure.
I propose to replace these methods with an alternate approach:
GvrsGridPoint gridPoint = spec.mapGeographicToGridPoint(latitude, longitude);
GvrsGeoPoint geoPoint = spec.mapGridToGeoPoint(gridPoint);
System.out.println("row: "+gridPoint.getRow());
System.out.println("column: "+gridPoint.getColumn());
I've tested these changes for performance and the new class-based technique is only a tiny bit slower than the array based approach (on average only about 1 part in 2397). In fact, in 18 our of 40 trials, the class-based technique was actually faster than the array-based form. So any differences are probably in the noise.
Updates were added for release 1.0.1. Old-style methods were deprecated, but retained.