Opened 12 years ago

Closed 9 years ago

#394 closed defect (fixed)

address discrepancy of hasPoint(x, y) behaviour in XY_DataSet implementations

Reported by: Kevin Milner Owned by:
Priority: major Milestone: OpenSHA 1.4
Component: commons Version:
Keywords: Cc:


the javadoc for XY_DataSet.hasPoint(x, y) describes the functionality as:

Determine wheither a point exists in the list, as determined by it's x-value within tolerance.

Most implementations only check that there is a value at the specified x value (which is how I interpret the javadoc). One exception, however, is EvenlyDiscretizedFunc? which checks that not only does a point exist at the given x value (within tolerance) but that the y value matches as well.

This is a point of potential confusion. Maybe we should have two different methods:

hasPoint(x, y)

The first would only check x values, the second would check both x and y values. We would have to decide what to return for hasPoint(Point2D), however. Thoughts?

Change History (6)

comment:1 Changed 12 years ago by Ned Field

Sounds good to me.

comment:2 Changed 12 years ago by Peter Powers

I did a casual survey and found that these methods are not extensively used (NoCollisionFunction? in Kevin's scratch and a test class I wrote for NSHMP use). I favor removing them entirely over adding new methods to clarify ambiguity.

If they must stay, I'd suggest refactoring to XY_DataSet.contains(Point2D) and then at the function level require DiscretizedFunction?.hasX(). Again, though, one can just as easily request the point and x and do one's own test to see if it's null.

I also noted that the unused DefaultXY_DataSet.hasPoint() simply defers to List.contains() and therefore does not consider 'tolerance', as the javadoc suggests.

As a final note, I added class javadoc comments to XY_DataSet indicating that the use of 'Set' does not imply adherence to java.util.Set.

comment:3 Changed 12 years ago by Peter Powers

think about precision

comment:4 Changed 10 years ago by Kevin Milner

Milestone: OpenSHA 1.3OpenSHA 1.4

don't want to make any big changes now, put to 1.4

comment:5 Changed 9 years ago by Kevin Milner

Done in [10917]. Now just a single hasX(double x), which has a default implementation in AbstractDiscretizedFunction? that returns getXIndex(double x) >= 0, and thus respects tolerance. Will close when rafactor branch is merged back in.

comment:6 Changed 9 years ago by Kevin Milner

Resolution: fixed
Status: newclosed

Fixes/updates merged to trunk in [10931], closing.

Note: See TracTickets for help on using tickets.