exercism / php-representer

GNU Affero General Public License v3.0
1 stars 2 forks source link

Normalize: Use float instead of double #146

Open mk-mxp opened 9 months ago

mk-mxp commented 9 months ago

I came across this in the PHP track docs:

  • Casts (float) and (real) are normalized to (double)

Was there a discussion about that anywhere? Because double is not seen by PHP as a type, it is seen as an alias for float only:

(double) and (real) are aliases of the (float) cast. These casts do not use the canonical type name and are not recommended.

Througout the PHP documentation float is used as the type for floating point numbers.

See:

github-actions[bot] commented 9 months ago

Hello. Thanks for opening an issue on Exercism 🙂

At Exercism we use our Community Forum, not GitHub issues, as the primary place for discussion. That allows maintainers and contributors from across Exercism's ecosystem to discuss your problems/ideas/suggestions without them having to subscribe to hundreds of repositories.

This issue will be automatically closed. Please use this link to copy your GitHub Issue into a new topic on the forum, where we look forward to chatting with you!

If you're interested in learning more about this auto-responder, please read this blog post.

homersimpsons commented 9 months ago

Reference forum post: https://forum.exercism.org/t/php-normalize-use-float-instead-of-double/9740

Note that this will require a new run of the representer. It will certainly make sense to batch it with other changes.

I did flag it as "paused", I had other improvements to include in the representer that I could write in new issues. Once all the merge are ready we could increase the representer version.

We should also update the documentation.

tomasnorre commented 8 months ago

If a tool like https://github.com/rectorphp/rector was introduced, changes like this, and PHP Upgrades to new versions later on, would be done quite easily.

homersimpsons commented 8 months ago

This change cannot be done with rector, technically this is a one-line change: https://github.com/exercism/php-representer/blob/d55173c8a8e80e4576da70783795b7aee9d4d9fb/src/NormalizeNodeVisitor.php#L205

- $node->setAttribute('kind', Double::KIND_DOUBLE);
+ $node->setAttribute('kind', Double::KIND_FLOAT);

Some tests and documentation has to be updated then.

As this representation is not displayed to any user and this change would require a new run of the representer I'm not in favour of doing it without including other "bigger" changes.

tomasnorre commented 8 months ago

OK. Fair, I understood it differently. Just ignore my comment.