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

Overload * operator for BitRate and Duration #968

Closed

Conversation

breckognize
Copy link

The change to overload the * operator for BitRate and Duration is simple, but doing so broke unit tests. To fix them, I needed to make Information a BaseDimension, which is the bulk of the diff. I believe I've done that successfully, but I confess I don't understand why "BaseDimensions" exists. (Does it enable a feature? Does it offer redundancy for testing?)

Breck Fresen added 3 commits September 18, 2021 00:34
This change adds Information, whose base unit is the bit, to
BaseDimensions. This enables a future change to overload the * operator
for BitRate that allows for (e.g.) 1 MB/sec * 1 hour = 3600MB.
Separated to make reviewing the previous diff simpler. This can be
squashed with the previous change on merge.
This enables expressions of the form 1MB/sec * 1 hour = 3600MB.
@angularsen
Copy link
Owner

Hi and thanks for contributing. It seems you hit an edge case here, sorry about that.
Some quantities, like BitRate and RatioChangeRate, were incorrectly configured as dimensionless quantities.
I pushed a fix now: 7b60fac

If you could please try this PR again, you should really only have to add your BitRate.extra.cs file, no other changes should be necessary. Let me know if you still meet a wall.

As for the intent behind BaseDimensions, it was an experiment to see if we could support dynamic conversions like IQuantity * IQuantity where the intermediate calculations may involve (Length * Length * Length * Length) / Length that have no strongly typed quantity for the representation inside the parantheses, but the end result is a Volume.

BaseDimensions is only intended for SI derived quantities, and as mentioned, it was an experiment we haven't fully explored or reaped the benefits from yet.

Main discussions
#651
#709

Other related topics from a quick search
#630
#524
#831
#588
#587
#180

Best, Andreas.

@stale
Copy link

stale bot commented Dec 9, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Dec 9, 2021
@stale stale bot closed this Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants