Volumes

[vitasimek]

Hi, there is an implementation of basic Volume units.

[angularsen]

Thanks for the contribution.

There are a few things I would like you to go over before I merge:
Please read Adding a Unit. I added it yesterday.

Volume.cs:

• I would choose Liter as the base unit instead of cubic meter, but that is just my preference. I am open to discuss this. If you agree, rename Meters field to Liters. This will serve as the base unit of volume.
• Rename Centimeters, Decimeters (and similar) properties and From methods to CubicCentimeters, CubicDecimeters and so on.
• Fix XML docs for Max and Min properties to say cubic meters instead of just meters.
• ToString() should show Liters + " L"

Unit.cs:

• Add enumeration for volume units. See wiki article.

UnitConverter.cs:

• Add two switch cases for new volume units. See wiki article.

I like the new additions to the unit tests. Similar tests should be added to the other units as well.

[vitasimek]

Hi Andreas,

I tried to think about main unit from the physics perspective. There at our homes we are focused on the volumes as a one litre of milk, beer, etc. But as an engineer you can be focused on the volume of a dam, planet. Things with a much bigger volume than the liter is suitable to express. On the other hand, the cubic meter is the basic unit as described on wikipedia. I don't know. I think that the most pragmatic way, how to determine the basic unit could be the balanced ranges for lower and upper ranges of double type for volume. But I don't know exactly how to perform it.
So, throw the dice, I will implement the result :)

Sry, I've made a bad decision with cubic prefix. The reason was, that the shortcut for cubic meter is cm that also means centimeter and I wanted to keep the style of using shortcuts there in method parameter name's. In this case I think it woul'd be better to use whole word: cubicMeter, etc.Wouldn't it? I am happy with cubic prefixes there.

Thanks for the wiki article, I will apply the news there.

One more thing to the operators. I was unsure with == and != operators. Should they be implemented as well or not?

Best Regards,
Víťa

[angularsen]

You may be right. If meter is the default for length, then it is
probably best that m² and m³ are default for area and volume as well.
You should go with that.

Yes, I think == and != should be implemented too. I have probably not
done so myself, but I think that is a good thing to do.

Best regards,
Andreas

On 23.07.2013 21:19, vitasimek wrote:

Hi Andreas,

I tried to think about main unit from the physics perspective. There
at our homes we are focused on the volumes as a one litre of milk,
beer, etc. But as an engineer you can be focused on the volume of a
dam, planet. Things with a much bigger volume than the liter is
suitable to express. On the other hand, the cubic meter is the basic
unit as described on wikipedia. I don't know. I think that the most
pragmatic way, how to determine the basic unit could be the balanced
ranges for lower and upper ranges of double type for volume. But I
don't know exactly how to perform it.
So, throw the dice, I will implement the result :)

Sry, I've made a bad decision with cubic prefix. The reason was, that
the shortcut for cubic meter is cm that also means centimeter and I
wanted to keep the style of using shortcuts there in method parameter
name's. In this case I think it woul'd be better to use whole word:
cubicMeter, etc.Wouldn't it? I am happy with cubic prefixes there.

Thanks for the wiki article, I will apply the news there.

One more thing to the operators. I was unsure with == and !=
operators. Should they be implemented as well or not?

Best Regards,
Víťa

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

[vitasimek]

Hi Adreas,
Today I've learned how to perform "get latest version" on GIT :) After a merge, I can see the new code there.
There is a class UnitConverter and UnitConversionTests - may I write some test for Volume conversions there into that test file, or is that only a similar name but different purpose? I can not fully understand that test file, so I am suspicious with that.

[angularsen]

Nice :-) I am also quite new to git, so still learning every day.
The purpose of UnitConversionTests is conversion across different
compatible classes of units, such as converting from weight force in
newtons to weight mass in kilograms.
The VolumeTests.cs you have already written should be sufficient I think.

The purpose of UnitConverter is to dynamically convert between units,
either from UnitValue.TryConvert() or by UnitConverter.TryConvert(). The
idea is to support cases where you don't know the type when you are
writing the code, so it relies on runtime checking of units to apply the
correct conversions.

On 23.07.2013 23:02, vitasimek wrote:

Hi Adreas,
Today I've learned how to perform "get latest version" on GIT :) After
a merge, I can see the new code there.
There is a class UnitConverter and UnitConversionTests - may I write
some test for Volume conversions there into that test file, or is that
only a similar name but different purpose? I can not fully understand
that test file, so I am suspicious with that.

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

[angularsen]

This looks very good now, are you done with your changes?

[vitasimek]

I'd like to implement the basic imperial units, but I can't guarantee, that it will be finished today. So if you accept current state with basic metric units, it's done.

[angularsen]

Sure, you can add the imperial units in the same pull request. I'm out this weekend and won't be able to take a look until Sunday at the earliest.

[angularsen]

Please note, I took inspiration from your unit tests and implemented it into all the existing unit classes.

Please add the following changes to this pull request:

• A few test methods regarding exceptions, compareto and equals were renamed, see LengthTests.cs for an example.
• Dynamic conversion tests using UnitConverter should be moved to UnitConverterTests.cs
• Dynamic conversion tests using UnitValue class should be moved to UnitValueTests.cs

[angularsen]

I also had to rewrite UnitConverter.cs somewhat to use TryConvert pattern all the way. I found a bug where TryConvert did not return false as expected, but rather threw an exception if the units were incompatible.
You will have to modify your TryConvert and Convert methods for Volume units to follow the same pattern, basically converting the Convert method to a TryConvert with an out parameter and returning bool.

[vitasimek]

I had to make quite complex merge and to keep the code compilable the tryconvert stuff was implemented together with that merge.

[angularsen]

Merged it. Fixed some misspelling of Millimeter and Milliliter, also changed the unit test to match the other unit tests.

[angularsen]

If you later add the imperial units just issue another pull request. Thanks!

[angularsen]

New nuget 1.5.0 with volume units is now out.
https://www.nuget.org/packages/UnitsNet/