Skip to content

QuantityType is not extensible #858

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

Closed
generateui opened this issue Nov 22, 2020 · 4 comments
Closed

QuantityType is not extensible #858

generateui opened this issue Nov 22, 2020 · 4 comments

Comments

@generateui
Copy link
Contributor

I'm currently adding some quantities with units for a specific domain. I miss a few things which I ideally like to see, most of these are convenience features which I can happily work around.

I have added .json files with specification into my own project. In there, I have defined quantities. I then wrote a script to copy these json files to the Common/UnitDefinitions folder of UnitsNet project. I subsequently ran the CodeGen assembly.

Changes made to the UnitsNet project make mostly sense.

One thing which does not make sense to me is the use of QuantityType as enum. I can't just add a QuantityType member - enums are not extensible. Note that the guide refers to HowMuch quantity which returns undefined.

An alternative would be to make QuantityType an interface IQuantityType with static instances.

  1. To be sure: am I expected to return QuantityType.Undefined like HowMuch?
  2. Can we make QuantityType extensible in some way?
@angularsen
Copy link
Owner

Hi and thanks for the detailed description.

Custom units is the least mature part of this library, and as you found out the enum is not extensible.
I think your suggestion of using classes is a good one.

If you are interested in attempting a pull request then I'd be happy to assist!

@generateui
Copy link
Contributor Author

I'd like to, but do not have the time currently. I need to dive in it for a day or so to have a good PR. It'd be good to discuss the design upfront, that may save some effort when prototyping/implementing.

I see the System.Enum type is used as surrogate supertype for Unit enums. Is this done for performance?

To represent QuantityType, I'd use an interface. Then have a class implement it with static instances. Do you avoid trying to instantiate static instances for performance?

I do not see performance listed as a specific feature. If it is not a feature, why not simply merge {Name}Type and {Name} for both Quantity and Unit into a class? A wrapper for QuantityValue seems logical.

@angularsen
Copy link
Owner

I see the System.Enum type is used as surrogate supertype for Unit enums. Is this done for performance?
Do you avoid trying to instantiate static instances for performance?

No, it just seemed like a good idea at the time. We had a bunch of LengthUnit, MassUnit etc enum types and we needed a generic way to map LengthUnit.Meter and LengthUnit.Centimeter to the abbreviations m and cm via dictionary lookups. Performance was not a particular reason for this design, it just kind of worked well enough.

why not simply merge {Name}Type and {Name} for both Quantity and Unit into a class?
A wrapper for QuantityValue seems logical.

I'm not 100% sure I follow, could you please take some example quantities like Length and Mass and some units like centimeter and kilogram and write a bit of pseudo code for the new classes and interfaces you propose to add as well as some example usages to illustrate your idea?

I understand you propose replacing enum QuantityType with values Length and Mass with static objects of IQuantityType. What would the properties of that interface be? How should it be extended with new quantities?
These sorts of things.

Best,
Andreas

@stale
Copy link

stale bot commented Feb 2, 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 Feb 2, 2021
@stale stale bot closed this as completed Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants