Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Add a File::bytesToHuman() helper #48827

Closed
wants to merge 3 commits into from

Conversation

caendesilva
Copy link
Contributor

@caendesilva caendesilva commented Oct 26, 2023

Abstract

Adds a File::bytesToHuman() helper to format the number of bytes to a human-readable string.

Functionality

The helper works by iterating through the input byte count and dividing it by 1024. The iteration count is the used in a lookup array to get the appropriate byte suffix. It supports rounding to a specific precision. Results are also slightly rounded up.

Development

I was originally thinking this could be a helper in the Str class, but after bringing it up in #internals we thought it might be better in the File facade. I am of course more than happy to move it to the Str class. Thanks to all the great people in the Discord!

Added tests

I added a test with 8 assertions to get a broad range of conditions. I also ran some benchmarks on different implementations, and all of them have minuscule performance concerns (~0.00058ms avg/1 million iterations).

Backwards compatibility

This adds a new feature, so it will not break any existing features. It can thus also go in a minor release version.

How this improves Laravel

This is one of those things I often add to Laravel applications as a Str macro. And I thought it is so useful, so I wanted to share it with everyone!

@driesvints driesvints changed the title Add a File::bytesToHuman() helper [10.x] Add a File::bytesToHuman() helper Oct 26, 2023
$bytes /= 1024;
}

return round($bytes, $precision).' '.['B', 'KB', 'MB', 'GB', 'TB'][$i];
Copy link
Contributor

Choose a reason for hiding this comment

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

That will not work for Petabyte:

https://3v4l.org/aUKD7

So please use a fallback if we have more then the highest unit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also: Maybe use sprintf for text output. This should imo be more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing! I could increase the lookup array to Yottabytes? I can of course also add a fallback, or both. What do you think is best?

Will use sprintf as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I would like to see it expanded to yottabytes when it is irrelevant whether it is needed or not, it is there and the function can then.

Additionally, you always need a fallback, as there can always be sizes you don't care about in the code (even if the integer is limited, you wan't always have a fallback).

So the easiest way for a fallback is a break within the loop, if we reach a certain point (or use a for loop with a break if $bytes < 1024).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback!

7d04625 adds a cap so that the iterator stops dividing when reaching the end of the units array which now also supports extreme values. Also uses sprintf as requested. Also adds tests for large numbers.

Let me know if you want me to squash the commits!

Adds a cap so that the iterator stops dividing when reaching the end of the units array. Also uses `sprintf` as requested. Also adds tests for large numbers.
@taylorotwell
Copy link
Member

Hey @caendesilva - I started digging into what Rails does here.

https://api.rubyonrails.org/classes/ActionView/Helpers/NumberHelper.html

This may make sense as part of a new Number utility class like Str? Maybe we should pursue that instead?

@caendesilva
Copy link
Contributor Author

Hey @caendesilva - I started digging into what Rails does here.

https://api.rubyonrails.org/classes/ActionView/Helpers/NumberHelper.html

This may make sense as part of a new Number utility class like Str? Maybe we should pursue that instead?

@taylorotwell That sounds even better to me! I'd love to help. I'm in the #internals Discord if you want to chat! (username @Caen)

caendesilva added a commit to caendesilva/laravel-framework that referenced this pull request Oct 28, 2023
Ports laravel#48827 into the new `Number` utility class
taylorotwell added a commit that referenced this pull request Nov 15, 2023
* Create a new Number utility class

* Add a `Number::bytesToHuman()` helper

Ports #48827 into the new `Number` utility class

* Add a `Number::toHuman()` helper

This is unfortunately dependent on the `ext-intl`, as it wraps the NumberFormatter class.

* Use lowercase `k` for kilobytes

See #48845 (comment)

* Update Support package to suggest `ext-intl`

* Throw if extension is not installed when using NumberFormatter wrapper

As discussed in #internals, this seems to be a good compromise when the extension is not used. Instead of testing against the exception in the tests, I just skipped the test if the extension is missing, as that is what we do in the InteractsWithRedis testing trait.

* Add a `Number::toCurrency()` helper

* Make Number helper locale parameters null and default to App locale

This makes so that if no locale parameter is specified, the configured App locale is used.

* Add a `Number::format()` helper

Adds a locale-aware number formatting helper

* Fix number tests

Could not get Mockery to work, so this is my fix.

* Add a `Number::toPercent()` helper

I'm dividing the supplied value by 100 so that 50 = 50% as that's how Rails does it, and that's what Taylor linked to in his suggestion for this class. The default number formatter would consider 0.5 to be 50% and 50 to be 5000%. I'm not sure which option is best, so I went with the Rails format. https://api.rubyonrails.org/classes/ActionView/Helpers/NumberHelper.html#method-i-number_to_percentage

* Rename Number toHuman helper to spellout

We may want to remove it, as per #48845 (comment). But renaming it for now.

* Create new `Number::toHuman()` helper

Based on the Rails implementation, as requested by Taylor in #48845 (comment)

See https://api.rubyonrails.org/classes/ActionView/Helpers/NumberHelper.html#method-i-number_to_human

Uses the short scale system, see https://en.wikipedia.org/wiki/Long_and_short_scales

* Change toHuman implementation to better match Rails version

Based more on the logic of Rails, but with added support for massive numbers.

* Update toHuman helper to better handle extreme numbers

Inverts negative numbers, and removes unreachable cases, and handles very large numbers

* Clean up toHuman helper

* formatting

* formatting

* formatting

* formatting

* formatting

---------

Co-authored-by: Taylor Otwell <taylor@laravel.com>
taylorotwell added a commit to illuminate/support that referenced this pull request Nov 15, 2023
* Create a new Number utility class

* Add a `Number::bytesToHuman()` helper

Ports laravel/framework#48827 into the new `Number` utility class

* Add a `Number::toHuman()` helper

This is unfortunately dependent on the `ext-intl`, as it wraps the NumberFormatter class.

* Use lowercase `k` for kilobytes

See laravel/framework#48845 (comment)

* Update Support package to suggest `ext-intl`

* Throw if extension is not installed when using NumberFormatter wrapper

As discussed in #internals, this seems to be a good compromise when the extension is not used. Instead of testing against the exception in the tests, I just skipped the test if the extension is missing, as that is what we do in the InteractsWithRedis testing trait.

* Add a `Number::toCurrency()` helper

* Make Number helper locale parameters null and default to App locale

This makes so that if no locale parameter is specified, the configured App locale is used.

* Add a `Number::format()` helper

Adds a locale-aware number formatting helper

* Fix number tests

Could not get Mockery to work, so this is my fix.

* Add a `Number::toPercent()` helper

I'm dividing the supplied value by 100 so that 50 = 50% as that's how Rails does it, and that's what Taylor linked to in his suggestion for this class. The default number formatter would consider 0.5 to be 50% and 50 to be 5000%. I'm not sure which option is best, so I went with the Rails format. https://api.rubyonrails.org/classes/ActionView/Helpers/NumberHelper.html#method-i-number_to_percentage

* Rename Number toHuman helper to spellout

We may want to remove it, as per laravel/framework#48845 (comment). But renaming it for now.

* Create new `Number::toHuman()` helper

Based on the Rails implementation, as requested by Taylor in laravel/framework#48845 (comment)

See https://api.rubyonrails.org/classes/ActionView/Helpers/NumberHelper.html#method-i-number_to_human

Uses the short scale system, see https://en.wikipedia.org/wiki/Long_and_short_scales

* Change toHuman implementation to better match Rails version

Based more on the logic of Rails, but with added support for massive numbers.

* Update toHuman helper to better handle extreme numbers

Inverts negative numbers, and removes unreachable cases, and handles very large numbers

* Clean up toHuman helper

* formatting

* formatting

* formatting

* formatting

* formatting

---------

Co-authored-by: Taylor Otwell <taylor@laravel.com>
@amorim778
Copy link

how to install this update?

@caendesilva
Copy link
Contributor Author

how to install this update?

A version of it will be released tomorrow! https://laravel-news.com/laravel-number-utility-class

@badasukerubin
Copy link

I assume that there could be a reverse sizeToBytes or humanToBytes method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants