GeoTools

Replace GeodeticCalculator.getGeodeticCurve

Details

  • Type: Improvement Improvement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: referencing
  • Labels:
    None
  • Testcase included:
    yes

Description

GeodeticCalculator.getGeodeticCurve returns a Shape. A List of Point2Ds would be much more convenient and also give double, rather than float, precision.

The attached patch deprecates getGeodeticCurve and adds a new method:
List<Point2D> getPath(int numPoints)

The patch also adds a private helper method computePoint to avoid the confusing dual use of computeDestinationPoint in the original code.

Unit tests are included but they could be better.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Andrea Aime added a comment -
Err... geodetic is not the only possible path. getGeodeticPath() maybe?
Show
Andrea Aime added a comment - Err... geodetic is not the only possible path. getGeodeticPath() maybe?
Hide
Permalink
Michael Bedward added a comment - - edited
No, but it's the only kind of path that GeodeticCalculator returns and having "geodetic" in the class and method name seemed a bit much. But I don't really care :-) Your call.
Show
Michael Bedward added a comment - - edited No, but it's the only kind of path that GeodeticCalculator returns and having "geodetic" in the class and method name seemed a bit much. But I don't really care :-) Your call.
Hide
Permalink
Andrea Aime added a comment -
Well, yes and no, there is also the loxodrome method, but as the comment say it's not working
Show
Andrea Aime added a comment - Well, yes and no, there is also the loxodrome method, but as the comment say it's not working
Hide
Permalink
Andrea Aime added a comment -
I also noticed a number of variables have been renamed.
Now, I agree the new names look better, but having a 1-1 resemblance with the original code is also a feature, if we ever find a problem we can quickly and easily compare with the source of the algorithm... so I would keep them as they are.
Show
Andrea Aime added a comment - I also noticed a number of variables have been renamed. Now, I agree the new names look better, but having a 1-1 resemblance with the original code is also a feature, if we ever find a problem we can quickly and easily compare with the source of the algorithm... so I would keep them as they are.
Hide
Permalink
Michael Bedward added a comment -
I'm going to attempt to change your mind :)

No fields were renamed, only local variables whose names clashed with field names. Having class and method vars with matching names makes for error prone code. Martin favoured matching names in the code to those used in the published references but I think its confusing.
Show
Michael Bedward added a comment - I'm going to attempt to change your mind :) No fields were renamed, only local variables whose names clashed with field names. Having class and method vars with matching names makes for error prone code. Martin favoured matching names in the code to those used in the published references but I think its confusing.
Hide
Permalink
Michael Bedward added a comment -
Should we close this one as 'will not fix' ? I don't actually remember much about it :)
Show
Michael Bedward added a comment - Should we close this one as 'will not fix' ? I don't actually remember much about it :)

People

  • Assignee:
    Andrea Aime
    Reporter:
    Michael Bedward
Vote (0)
Watch (0)

Dates

  • Created:
    Updated: