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

Equally use singular for unit types as in the c# project #29

Closed
Shentoza opened this issue Mar 7, 2024 · 10 comments
Closed

Equally use singular for unit types as in the c# project #29

Shentoza opened this issue Mar 7, 2024 · 10 comments

Comments

@Shentoza
Copy link

Shentoza commented Mar 7, 2024

We're using the unitsnet-js as a way to have the same information in our frontend, that we also have in our backend.
However in this package all unitnames are in plural (e.g. Meters, Millimeters, Inches ) while in the c# project they are singular (e.g. Meter, Millimeter, Inch ) which makes conversion between our c# backend and our typescript frontend really painful.
Is this a conscious decision?

@haimkastner
Copy link
Owner

Hi @Shentoza

Are you referring to the unit's enum? LengthUnits in the length unit case?

This enum convention is not the same as in the C#, and I do agree that it was better to be the same convention, but why is this so painful?

Can you please share a use case for why it's so matter?

I can think of a use case, when you want to pass the unit as part of an API response, for instance:

{
 "value": 10,
 "unit": "Meters"
}

Then in the consumer, you want to do the following:

const res = api_call()
const length = new Length(res.value, res.unit)

But, if this is the case, it doesn't matter what the enum key name is, but what the actual value is.

BTW, currently, the enums value of the units are random numbers (depending on the order, auto set number)

@Shentoza
Copy link
Author

Shentoza commented Mar 8, 2024

Because the object coming from my Backend is

{ 
    value: 10,
    unit: "Meter"
}

I'm using the enum name here to have a more speaking response in case anyone not using UnitsNet is calling my API, where the enum value of e.g. 12 would be confusing.
Now i have to do a conversion where I map the singular response of the backend to the plural naming of the typescript enum, s.t. I can create an Length object, (since there is only an constructor(value: number, fromUnit?: LengthUnits); constructor I need the actual enum value. And if I want to send the object back to the backend, I need to convert it back again to the singular form.

I have seen that in the generator files ( unit-generator.ts ) in buildEnum it is specified to take the plural name, which should be easy to alter, no?

@Shentoza
Copy link
Author

Shentoza commented Mar 8, 2024

For now I defined a lookup table that converts the units to their singular equivalent

const enumToSingular = {
  [LengthUnits.Meters]: 'Meter',
  [LengthUnits.Miles]: 'Mile',
  [LengthUnits.Yards]: 'Yard',
[...]

which works but is more of a workaround

@haimkastner
Copy link
Owner

I understand, so basically if I will change the value of the enum from some random number to the unit non-plural name

Like this one
https://github.com/haimkastner/unitsnet-js/blob/master/src/length.g.ts#L4

Meters = 'Meter',

It will work for you even if the enum key is still plural, like this:

const length = new Length(res.value, res.unit as LengthUnits)

I have seen that in the generator files ( unit-generator.ts ) in buildEnum it is specified to take the plural name, which should be easy to alter, no?
Right, it's easy to change it, but it will break the package API (unless I will duplicate it and deprecate the old unit enumas I explain above).

@Shentoza
Copy link
Author

Shentoza commented Mar 8, 2024

I understand, so basically if I will change the value of the enum from some random number to the unit non-plural name

Like this one https://github.com/haimkastner/unitsnet-js/blob/master/src/length.g.ts#L4

Meters = 'Meter',

It will work for you even if the enum key is still plural, like this:

const length = new Length(res.value, res.unit as LengthUnits)

That would be great!

@haimkastner
Copy link
Owner

@Shentoza
The new version 2.0.1 contains the fix from #30 published and ready in NPM.

@haimkastner
Copy link
Owner

@Shentoza
Regarding your use-case WDYT about #31?

@Shentoza
Copy link
Author

Shentoza commented Mar 8, 2024

@Shentoza The new version 2.0.1 contains the fix from #30 published and ready in NPM.

Thank you so much. We're using yarn, is the package already pushed there as well? im taking a look into #31

@haimkastner
Copy link
Owner

@Shentoza
Yes, AFAIK yarn is doing uplink to npm.

@Shentoza
Copy link
Author

Shentoza commented Mar 8, 2024

Yes, sorry I have to upgrade --latest since it's a new major version, which I forgot

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

No branches or pull requests

2 participants