Skip to content

Conversation

@N-Dekker
Copy link
Contributor

Modernized the code. Also improved the style, by removing static_cast<float>s.

Modernized the code. Also improved the style, by removing `static_cast<float>`s.
@github-actions github-actions bot added type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module type:Style Style changes: no logic impact (indentation, comments, naming) labels May 14, 2025
@N-Dekker N-Dekker marked this pull request as ready for review May 14, 2025 13:51
Comment on lines -42 to +44
auto randMax = static_cast<float>(RAND_MAX);
const PixelType pixel(static_cast<float>(rand()) / randMax, static_cast<float>(rand()) / randMax);
const PixelType pixel(randomNumberDistribution(randomNumberEngine), randomNumberDistribution(randomNumberEngine));
Copy link
Contributor Author

@N-Dekker N-Dekker May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, this pull request was inspired by a discussion we had before about the use of static_cast<float>(RAND_MAX), at #5337 (comment) It appeared that RAND_MAX is platform dependent, so this static_cast<float> may or may not be lossless. Which made me want to get rid of static_cast<float>(RAND_MAX) 👹 !

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Niels! 🥇

@thewtex thewtex merged commit facf527 into InsightSoftwareConsortium:master May 14, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants