Improve usability and consistency of spatial API
Description
relates to
Activity
Sanne Grinovero November 11, 2014 at 3:09 PM
Sounds good. Thanks for reviewing and distilling all this information!
Hardy Ferentschik November 10, 2014 at 8:31 PM
AFAICT everything in this issue has been already addressed. It was decided that Point is an implementation class and should not be used by user code. This was addressed by HSEARCH-1315.
This leaves a DistanceSortField and FullTextQuery#setSpatialParameters where the idea is to provide methods which don't need a explicit field name, but rather act upon the default spatial (field). The issue here, in particular with FullTextQuery, is the amount of overloaded methods.
According to @Kalgon:
If a constant for the default field name is available in a public (non-impl) location, then I wouldn't need the new methods which don't take the field name.
COORDINATES_DEFAULT_FIELD has actually already been moved into public API namely Spatial#COORDINATES_DEFAULT_FIELD. So this is covered as well. The only thing which worries me a bit with this, is that Spatial#COORDINATES_DEFAULT_FIELD is not terribly obvious. It is not even mentioned in the docs.
Bottom line, unless we want to add yet another method to FullTextQuery, I think we have covered everything. I'd suggest adding a note to the docs though regarding Spatial#COORDINATES_DEFAULT_FIELD though. Thoughts?
Hardy Ferentschik November 10, 2014 at 8:09 PM
Point was removed from the public API (FullTextQuery) in https://hibernate.atlassian.net/browse/HSEARCH-1315#icft=HSEARCH-1315
Nicolas Helleringer May 14, 2013 at 3:24 PM
As stated in HSEARCH-1315, I ll move Point reference in setSpatialParameters to Coordinates
The case of the Point class is not an easy one.
It does exists to free Hibernate Search from integrating a full geo distance / projection computation lib.
But as it is not the purpose of Hibernate Search to support such a class for public use, it was decided to keep it private.
Moving COORDINATES_DEFAULT_FIELD to public API seems quite logical to me.
Sanne, what do you think ?
Xavier Dury February 14, 2013 at 10:22 AMEdited
Hi,
Indeed the usage of Point|Coordinates is not clear: as stated in the doc, my entities can implement Coordinates so it is part of the public API and is normally safe to use where I want/need.
Point is indeed in a impl package but then why is it used in FullTextQuery.setSpatialParameters(Point, String)? Shouldn't it be FullText.setSpatialParameters(Coordinates, String)? If the usage of Point is removed from the public API, then I won't use it anymore.
By the way, wouldn't it be better to have something like SpatialConstants.COORDINATES_DEFAULT_FIELD instead of AbstractDocumentBuilder.COORDINATES_DEFAULT_FIELD? If a constant for the default field name is available in a public (non-impl) location, then I wouldn't need the new methods which don't take the field name.
Details
Details
Assignee
Reporter
Labels
Components
Fix versions
Priority
Trivial
SpatialContext has two methods:
onDefaultCoordinates()andonCoordinates(String), the first one makes it easy for simple cases, butDistanceSortFieldconstructors must know the spatial field name (you can't create one for the default coordinates). I suggest to addDistanceSortField(Point)andDistanceSortField(double, double)which act on default coordinates.Same for
FullTextQuery.setSpatialParameters(Point, String)andFullTextQuery.setSpatialParameters(double, double, String). There should beFullTextQuery.setSpatialParameters(Point)andFullTextQuery.setSpatialParameters(double, double).I would like to reuse distance computation in my own code: if my entity implements
Coordinatesand I have aPointorigin, I would like to be able to callorigin.getDistanceTo(myEntity)butPoint.getDistanceTo()takes aPoint, not aCoordinates. I suggest to changePoint.getDistanceTo(Point other)toPoint.getDistanceTo(Coordinates other).