EngineeringUnits icon indicating copy to clipboard operation
EngineeringUnits copied to clipboard

Adding Cost/Currency/Price

Open MadsKirkFoged opened this issue 5 months ago • 6 comments

Creating a Issue based in @mschmitz2's excellent suggestion in #19 to add cost. It has been on my list for some time now so lets give it a go!

I have added following (still subject to change..)

Cost f1 = new Cost(10, CostUnit.USDollar); //10 $
Energy e1 = new Energy(10, EnergyUnit.KilowattHour); //10 kWh

EnergyCost test = f1 / e1; //1 $/kWh

Any other name we should consider for the unit representing $£€? Price Cost Currency

Name when combining it with other units: EnergyCost CostOfEnergy EnergySpecificCost EnergyPrice

What do you guys think?

Also, we need to figure out how to convert between the different Currencies.

  • Have a hardcoded conversion that will (automatic) be updated with each build.
  • Have a way you can override the hardcoded conversions with your own
  • an option to Connect to a public Currency exchange API that will give you the current currencies

MadsKirkFoged avatar Jan 16 '24 13:01 MadsKirkFoged

Any other name we should consider for the unit representing $£€? Price Cost Currency

Personally, I think Cost is enough, but maybe there are applications where people might prefer one of the other so adding an alias might be OK.

We should just keep in mind that, unlike with aliases of combined units, having an alias for a base unit might add either

  • potential for confusion (e.g. user defines a Price and an Energy, the result of the division of the two has units EnergyCost; EnergyPrice does not exist) for combined units, or
  • complexity (do we then need aliases of all derived combined units, i.e. both EnergyCost and EnergyPrice etc?), number of derived units can increase exponentially.

Name when combining it with other units: EnergyCost CostOfEnergy EnergySpecificCost EnergyPrice

I think I chose EnergySpecificCost because we already have units like SpecificEnergy etc. but I don't necessarily think it's the best.

I do like your suggestion of EnergyCost, mostly because it's the shortest. I also like CostOfEnergy as it would nicely group all "CostOf" units together in alphabetic order.

Also, we need to figure out how to convert between the different Currencies.

  1. Have a hardcoded conversion that will (automatic) be updated with each build.
  2. Have a way you can override the hardcoded conversions with your own
  3. an option to Connect to a public Currency exchange API that will give you the current currencies

This is probably the trickiest point.

  • With Option 1. is that you'd then probably also want to set up automatic builds, say every 1 month or week, to update the currencies regardless of whether there have been other builds. But you'd also kind of be forcing the user to update NuGet more regularly then they'd otherwise would.
  • I think Option 2. needs to be added if you have Option 1., mainly to avoid the problem of regular updates described above but also to allow e.g. to simulate conversion rates from the past or set fixed rates to keep calculations comparable to older versions
  • Option 3. would be nice but probably the most complicated. Again, needs to be combined with Option 2. to avoid issues described above.

mschmitz2 avatar Jan 16 '24 15:01 mschmitz2

I also like Cost and EnergyCost even thought some of the unit might end up sounding a bit funny; LengthCost, VolumeCost, AreaCost. But I think most people will understand the intent. I agree with not creating aliases for this, it will mean making too many new units. I think will create option 1 and 2. This way uses that just want ballpark currency conversions can use the build-in rates while others can input their own

MadsKirkFoged avatar Jan 16 '24 19:01 MadsKirkFoged

@mschmitz2 I have added all the Cost units that you were using.

Feel free to make a pull request where you add more unit quantities: image

MadsKirkFoged avatar Jan 19 '24 09:01 MadsKirkFoged

Thanks. I added a few units quantities and also fixed an issue where the namespace of the new units (*Enum.cs files) was EngineeringUnits instead of EngineeringUnits.Units like in the older units. This resulted in the getters and setters not being generated for the new classes.

I wanted to open a pull request for this but I'm having trouble publishing my branch... image

I am assuming the issue is on my side, but just in case, could you check if there might be any repo setting preventing me from doing this? Thanks

mschmitz2 avatar Jan 19 '24 11:01 mschmitz2

I think you need to:

  1. fork a new version.
  2. Clone that version to visual studio
  3. Create the changes
  4. Push to your forked version which you have access to.
  5. Go to github.com -> your fork -> Create pull request

Then I can accept your pull request if im happy with it ;-D

MadsKirkFoged avatar Jan 23 '24 07:01 MadsKirkFoged

Oh I see.. I was attempting this without forking (cloned, made my own branch, then tried to push that, which I don't have permission to do). With forking this works fine, as you can see in the pull request above

mschmitz2 avatar Jan 23 '24 14:01 mschmitz2