Feature Proposal: Humanize ByteSize to rate ("1 KB/s")

[GeorgeHahn]

Proposed API: 1.Megabytes().PerSecond().Humanize(); // 1 MB/s

Questions:
Is this feature desirable?
Are intervals other than one second desirable? (If so, what are some example use cases?)

Off the top of my head, the implementation could be a PerSecond extension method for ByteSize that would return a ByteSizeRate class. This would hold a ByteSize and Humanize by appending "/s" to the Humanized ByteSize.

[MehdiK]

Thanks @GeorgeHahn. I don't know. I don't feel it adds much to be honest with you.

Humanizer is just a collection of extension methods which means you can very easily extend it yourself. For example you could create a RatePerSecond extension method on ByteSize that does that: 1.Megabytes().RatePerSecond() => 1 MB/s.

What do others think?

[justin-edwards]

Might be useful to have something like x.Megabytes().Per(timeSpan).ToRatePerSecond().

[hazzik]

👍 for @justin-edwards

[MehdiK]

Sorry @justin-edwards & @hazzik. I don't quite get the API. Why do we pass the timespan in and what's the expected result? Can you provide a complete example? I mean we're calling ToRatePerSecond so we already know it's per second, and as @GeorgeHahn mentioned, I can't think of an example where 3 MB/m (minute) or /h (hour) or /d (day) or /w (week) is useful.

[mexx]

👍 for @justin-edwards

@MehdiK The use case would be like this:
You measure the size of the download and track the time spent and wants to display the average download speed to the user.
With var averageSpeed = downloadSize.Bytes().Per(timeSpent).ToRatePerSecond() you will get your result.

About other time units, /m /h /d /w, they can be useful for displaying statistical averages, but I don't know how often it's needed. If we decide to go with only /s option, I would drop the extra call to .ToRatePerSecond().

Just my 2c

[MehdiK]

Cool. I really like it.

I agree with @mexx about removing the extra method call.

[justin-edwards]

Maybe .Humanize() rather than .ToRatePerSecond() since that keeps it more consistent with the rest of Humanizer and allows for the potential to add optional params to specify the units to be used down the road.

[GeorgeHahn]

Just implemented this.

@justin-edwards, I tried with .Humanize() rather than .ToRatePerSecond(), but that makes the code somewhat nonobvious.

var rate = size.Per(interval).ToRatePerSecond();
tells a lot more than
var rate = size.Per(interval).Humanize();

[GeorgeHahn]

Sorry, ignore the first commit - I forgot commit -a doesn't add new files.

[mexx]

@GeorgeHahn cool you tackle it.
You're right about .Humanize() vs .ToRatePerSecond(), but I would try to change the API in a way to use Humanize for unification matter.

What do you think about
var rate = size.ToRatePer(interval).Humanize();

[GeorgeHahn]

I'm worried about situations where that could be confusing:
size.ToRatePer(TimeSpan.FromMinutes(1)).Humanize() for example, would read like it's getting the rate per minute.

Perhaps size.ToRatePerSecond(interval).Humanize();? This has the added benefit of allowing interval to be optional and default to 1 second.

That enables this:
size.ToRatePerSecond().Humanize(); -> 60 MB/s
as well as this:
size.ToRatePerSecond(TimeSpan.FromMinutes(1)).Humanize(); -> 1 MB/s

[mexx]

There are two intervals involved, one of the measurement (any interval) and one for display (most likely a fixed unit of time).

With the second thought I would stick to

var rate = size.Per(measurementInterval);
var text = rate.Humanize();

And with an optional parameter timeUnit to Humanize we can control the display unit of time.

var text = rate.Humanize(timeUnit: TimeUnit.Hour);

I think it only reads non-obvious when you write it without extra rate variable.

[MehdiK]

I am torn on this tbh; but I like @mexx's suggestion a bit better. We could also provide both methods on ByteRate! :)

[GeorgeHahn]

I agree! Having both intervals makes a lot of sense; I'm not psyched about the codefeel/readability, but I think it will work out!

[GeorgeHahn]

Implemented. Here's how it looks:
var text = size.Per(measurementInterval).Humanize();
-> 10 MB/s

var rate = size.Per(measurementInterval);
var text = rate.Humanize(TimeUnit.Minute);

-> 10 MB/min

var text = size.Per(measurementInterval).Humanize(TimeUnit.Hour);
-> 10 MB/hour

I've only implemented H/M/S TimeUnits for now.

[MehdiK]

Cool. Please push your changes up to the PR and fix the issues @mexx raised. Also leave us a comment on the PR when that's done as we don't get notifications on commits. Thanks.

[GeorgeHahn]

Changes pushed, readme and changelog updated, doco written. Wasn't able to check for R# warnings (uninstalled for C#6 compatibility). Let me know what you think.

[GeorgeHahn]

@mexx, did I ever thank you for finishing this up?

I tried running the ApiApprover test on my desktop and my laptop; neither worked, even after a fresh install of Win 8.1 on my desktop. I had pretty well abandoned hope for getting this in, so I really appreciate you taking care of it.

Thanks!

[MehdiK]

Sorry to hear it hurt! You should've let us know so we could help you with
it.

We need to find and fix the issue for your future contributions, and also
to get to the bottom of this in case it happens to others.

On Tue, Aug 12, 2014 at 9:25 AM, George Hahn [email protected]
wrote:

@mexx https://github.com/Mexx, did I ever thank you for finishing this
up?

I tried running the ApiApprover test on my desktop and my laptop; neither
worked, even after a fresh install of Win 8.1 on my desktop. I had pretty
well abandoned hope for getting this in, so I really appreciate you taking
care of it.

Thanks!

—
Reply to this email directly or view it on GitHub
#296 (comment).

[GeorgeHahn]

I can get you stacktraces and open an issue. Is there an ApiApprover
project I should open it against, or should I do it here?

On Mon, Aug 11, 2014 at 7:27 PM, Mehdi Khalili [email protected]
wrote:

Sorry to hear it hurt! You should've let us know so we could help you with
it.

We need to find and fix the issue for your future contributions, and also
to get to the bottom of this in case it happens to others.

On Tue, Aug 12, 2014 at 9:25 AM, George Hahn [email protected]
wrote:

@mexx https://github.com/Mexx, did I ever thank you for finishing
this
up?

I tried running the ApiApprover test on my desktop and my laptop;
neither
worked, even after a fresh install of Win 8.1 on my desktop. I had
pretty
well abandoned hope for getting this in, so I really appreciate you
taking
care of it.

Thanks!

—
Reply to this email directly or view it on GitHub
#296 (comment).

—
Reply to this email directly or view it on GitHub
#296 (comment).

[MehdiK]

There is ApiApprover https://github.com/JakeGinnivan/ApiApprover but I'd
like to confirm that it's not our issue before submitting the issue there.
So please submit an issue here with all the issues you faced.

Thanks a lot.

On Tue, Aug 12, 2014 at 9:36 AM, George Hahn [email protected]
wrote:

I can get you stacktraces and open an issue. Is there an ApiApprover
project I should open it against, or should I do it here?

On Mon, Aug 11, 2014 at 7:27 PM, Mehdi Khalili [email protected]
wrote:

Sorry to hear it hurt! You should've let us know so we could help you
with
it.

We need to find and fix the issue for your future contributions, and
also
to get to the bottom of this in case it happens to others.

On Tue, Aug 12, 2014 at 9:25 AM, George Hahn [email protected]
wrote:

@mexx https://github.com/Mexx, did I ever thank you for finishing
this
up?

I tried running the ApiApprover test on my desktop and my laptop;
neither
worked, even after a fresh install of Win 8.1 on my desktop. I had
pretty
well abandoned hope for getting this in, so I really appreciate you
taking
care of it.

Thanks!

—
Reply to this email directly or view it on GitHub
#296 (comment).

—
Reply to this email directly or view it on GitHub
#296 (comment).

—
Reply to this email directly or view it on GitHub
#296 (comment).