minor API change suggestion on pointPolygonTest() (Feature #4095)

Added by Sebastien Kramm about 10 years ago. Updated about 10 years ago.

Status:Cancelled Start date:2015-01-02
Priority:Normal Due date:
Assignee:- % Done:

0%
Category:-
Target version:-
Difficulty: Pull request:

Description

I suggest an API change for the function:

cv::pointPolygonTest()

See doc: http://docs.opencv.org/modules/imgproc/doc/structural_analysis_and_shape_descriptors.html#double%20pointPolygonTest%28InputArray%20contour,%20Point2f%20pt,%20bool%20measureDist%29

It returns a `double` that can be either -1,0 or +1. This induces bad code practices, as the user will probably do things like:

    if( 1.0 == cv::pointPolygonTest() )

Which is obviously a bad idea, as floating-point values should never be compared with the == operator.

I checked the current source:
https://github.com/Itseez/opencv/blob/master/modules/imgproc/src/geometry.cpp

(line 95)
This value can in some situation be computed as a square root (line 235). But if this computed value is always 0 or 1, then why not return an int ?

Or maybe the 'double' is intended for some future extension ?
Or maybe the general policy is not to break the API ? (although I doubt in the present case that it would break anything)

Associated revisions

Revision 7bfd0708
Added by Maksim Shabunin over 9 years ago

Merge pull request #4095 from alalek:hal_unresolved_symbols

History

#1
Updated by Sebastien Kramm about 10 years ago

  • Status changed from New to Cancelled

#2
Updated by Sebastien Kramm about 10 years ago

CANCELLED, SORRY FOR THE NOISE, DIDNT READ DOC ENOUGH !

Sebastien Kramm wrote:

I suggest an API change for the function:
[...]

See doc: http://docs.opencv.org/modules/imgproc/doc/structural_analysis_and_shape_descriptors.html#double%20pointPolygonTest%28InputArray%20contour,%20Point2f%20pt,%20bool%20measureDist%29

It returns a `double` that can be either -1,0 or +1. This induces bad code practices, as the user will probably do things like:
[...]
Which is obviously a bad idea, as floating-point values should never be compared with the == operator.

I checked the current source:
https://github.com/Itseez/opencv/blob/master/modules/imgproc/src/geometry.cpp

(line 95)
This value can in some situation be computed as a square root (line 235). But if this computed value is always 0 or 1, then why not return an int ?

Or maybe the 'double' is intended for some future extension ?
Or maybe the general policy is not to break the API ? (although I doubt in the present case that it would break anything)

Also available in: Atom PDF