Deprecate the $num_points parameter of php_imagepolygon #4885
Conversation
That parameter is mostly useless in practise, and likely has been directly ported from the underlying `gdImagePolygon()` and friends, which require that parameter since the number of elements of the point array would otherwise be unknown. Typical usages of `imagepolygon()`, `imageopenpolygon()` and `imagefilledpolygon()` pass `count($points)/2` or hard-code this value as literal. Since explicitly specifying this parameter is annoying and error-prone, we offer the possibility to omit it, in which case the `$points` array must have an even number of elements, and the number of points is calculated as `count($points)/2`. We deprecate explicitly passing the `$num_points` parameter, but keep the sloppy behavior of the functions in this case (for instance, having an odd number of elements in `$points`).
|
I think this is a good change, having worked heavily with the GD API myself I always wondered why this was exposed in this way to userland, it is a thing we shouldn't burden userland with as we can easily calculate it ourselves. |
ext/gd/gd.stub.php
Outdated
| @@ -199,11 +199,11 @@ function imagecolortransparent(GdImage $im, int $col = UNKNOWN): ?int {} | |||
|
|
|||
| function imageinterlace(GdImage $im, int $interlace = UNKNOWN): ?int {} | |||
|
|
|||
| function imagepolygon(GdImage $im, array $points, int $num_pos, int $col): bool {} | |||
| function imagepolygon(GdImage $im, array $points, int $num_points, int $col = UNKNOWN): bool {} | |||
There was a problem hiding this comment.
| function imagepolygon(GdImage $im, array $points, int $num_points, int $col = UNKNOWN): bool {} | |
| function imagepolygon(GdImage $im, array $points, int $num_points_or_col, int $col = UNKNOWN): bool {} |
Maybe?
There was a problem hiding this comment.
That's definitely more correct for reflection, but might be confusing wrt. the deprecation warning ("the $num_points parameter is deprecated"). However, having an optional parameter in the middle of the parameter list is confusing anyway, but as it would only be temporary, guess it's okay.
|
My only concern here would be that this both introduces the new signature and deprecates the old one in the same version. I'd generally prefer it if the new form is introduced first and the deprecation happens later. |
|
I'm fine with postponing the deprecation (we should not forget about it, though). Anyhow, how to proceed here? Discussion on internals? |
|
Fine, thanks! Applied as 2de79f0. |
Cf. <php/php-src#4885>. git-svn-id: https://svn.php.net/repository/phpdoc/en/trunk@352128 c90b9560-bf6c-de11-be94-00142212c4b1
That parameter is mostly useless in practise, and likely has been
directly ported from the underlying
gdImagePolygon()and friends,which require that parameter since the number of elements of the point
array would otherwise be unknown. Typical usages of
imagepolygon(),imageopenpolygon()andimagefilledpolygon()passcount($points)/2or hard-code this value as literal. Since explicitly specifying this
parameter is annoying and error-prone, we offer the possibility to omit
it, in which case the
$pointsarray must have an even number ofelements, and the number of points is calculated as
count($points)/2.We deprecate explicitly passing the
$num_pointsparameter, but keepthe sloppy behavior of the functions in this case (for instance, having
an odd number of elements in
$points).See also the internals discussion from a while back.