Humanizr / Humanizer

Humanizer meets all your .NET needs for manipulating and displaying strings, enums, dates, times, timespans, numbers and quantities
Other
8.62k stars 958 forks source link

ByteSize does not follow the IEEE 1541 standard #592

Open Alexander882 opened 7 years ago

Alexander882 commented 7 years ago

The ByteSize struct uses values such as kilobyte = 1024 byte, whereas the official IEEE 1541 standard dictates that a kilobyte = 1000 bytes, and kibibyte = 1024 bytes, and so on. Wikipedia Link.

Also, the SI unit kilo- is normally shortened with a lowercase k, not a capital K.

I will happily fix these issues myself, but it might be seen as a bit of a breaking change, so I thought I'd hear your opinion first.

aloisdg commented 7 years ago

Hello,

Thank you for pointing this. I think it should be corrected. If it is a breaking change and a good one, we will still add it, just later on.

Alexander882 commented 7 years ago

Okay, I'll fix it and send in a pull request with my changes then.

I've also realized a few other things about the ByteSize struct, especially the TryParse method:

aloisdg commented 7 years ago

Even better. Please dont forget to add unit tests to show all of this.

omar commented 7 years ago

Hey guys, I'm the author of ByteSize and peek into this repo every once in a while to see how others use the library and what changes they want.

Thank you for the feedback.

hangy commented 7 years ago

This is obviously a breaking change, as callers may depend upon the KB values being based on 1024 Bytes instead of 1000 Bytes for whatever calculations. But it could be added as a non-breaking change in a minor release by adding an AppContext feature switch. If the switch is off (default), then 1 KB is 1024 Bytes (current behaviour), and if it's on, then a 1 KB is 1024 Bytes.

Negative values: This is an interesting suggestion. I intentionally chose to disallow that behavior thinking it wouldn't be of use. Could you elaborate on a use case for this?

Negative values could be interesting to display soft disk/mailbox/traffic quota violations.

omar commented 7 years ago

@hangy, thanks for the suggestion. I went ahead and did a quick spike of the metric byte, see these lines and the associated test case.

I opted to use a static field on the actual class because it's more self-documenting than AppContext (which requires magic strings). In addition, AppContext is relatively new and is not supported on older versions of .NET.

In terms of negative values and quota violations, generally those are greater than violations. That is:

var quota = ByteSize.FromMegaBytes(20);
var usage = ByteSize.FromMegaBytes(19);
var newFile = ByteSize.FromMegaBytes(2);

if (usage + newFile > quota) {
    throw new InvalidOperationException($"This file would put you above your quota of { quota }");
}

However, I did just realize that you can't subtract two ByteSize objects. I'm not sure why I didn't include that operation so I can do something like this:

throw new InvalidOperationException($"This file would put you above your quota of { quota } by { usage + newFile - quota }");

I'll add that as well as multiplying and dividing. Lastly, ByteSize does store negative bytes, but it doesn't print them out or parse them, but that can be changed.

I'll add those.

clairernovotny commented 7 years ago

@omar @aloisdg @Alexander882 any chance we can get this nailed down in the next week or so for 2.2? Aiming to get 2.2 out in the beginning of May.

mattdwen commented 6 years ago

Hi all, any update on this? I'd like to help out with this one if I can as it's something I'd like to use.

Alexander882 commented 6 years ago

Sorry for being so slow on this one, I've created a pull request with my implementation

MehdiK commented 6 years ago

This is a significant breaking change, and like @omar mentioned above, while the current terminology is technically wrong, it is what most people are used to and expect.

Can I suggest that we keep the existing functionality as default behavior, to avoid breaking the behavior, and add a strategy to switch to IEEE standard through config? This is an example of a similar implementation.

omar commented 5 years ago

I went ahead and pushed an implementation at https://github.com/omar/ByteSize/pull/24 which can be downloaded from https://www.nuget.org/packages/ByteSize/2.0.0-alpha1

Full design reasoning here https://github.com/omar/ByteSize/blob/db3cacc8314a9f16f327e8ed0df205d3bc186fba/docs/binary-byte.md

Tentative Release Notes

Breaking Changes:

Thoughts?

sandrock commented 5 years ago

It looks like there exist chaos worshippers. Windows has long been misusing file size units and some guys love it this way. It is not gonna end soon. Now, I think the software industry must make a step and move forward towards non-ambiguous displays of sizes.

It's OK to make a breaking change. Those who will update their library can be notified and apply the changes (and they can also not update).

In 5, 10 or 20 years, will you tolerate that your worldwide-used library does not adhere to a most-needed standard? It's better to do the change now as it may be more difficult in the future.

And... Why not respect the standard from the beginning? An acceptable error that must be fixed ;)

beeradmoore commented 4 years ago

Just had an awkward time with this. Using it in a Xamarin.Forms project (iOS/Android) and trying to figure out why my iPhone as being "238 gigabytes" when it really means "238 gibibytes", and then digging around looking why iOS is possibly giving me an incorrect number of device storage size in bytes incorrectly.

Good to know traction from the multiple proposed/implemented solutions is dead in the water.

omar commented 4 years ago

@beeradmoore version 2 of ByteSize supports the proper naming and values, you can get it here https://www.nuget.org/packages/ByteSize/2.0.0-beta1

I plan on release v2 and dropping the beta tag this holiday break. Once that's done, it can be pulled into Humanizer. However, given the large breaking change, I'm not sure how the maintainers want to handle this update.

omar commented 4 years ago

v2 o ByteSize has been released with this feature. See https://www.nuget.org/packages/ByteSize/2.0.0.

Yomodo commented 2 years ago

Are there still plans to implement v2.0 of ByteSize in Humanizer?