Normalize Temperature arithmetic #550
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are instead handled by standard arithmetic, now generated for Temperature.
|
@Mobz87 If you could look over this too, that would be great! |
tmilnthorp
reviewed
Nov 6, 2018
| public static TemperatureDelta FromTemperatures(Temperature left, Temperature right) | ||
| { | ||
| // TemperatureDeltaUnit enum has identical values to TemperatureUnit, so we can simply cast between them | ||
| return new TemperatureDelta(left.Value - right.As(left.Unit), (TemperatureDeltaUnit)left.Unit); |
There was a problem hiding this comment.
A straight enum cast relies on the ordering in the json file. Perhaps a switch would be better?
There was a problem hiding this comment.
I guess, I figured since they were generated it would be safe. How about instead enum value ToString() and enum parse? Then it will throw if they are not identical.
There was a problem hiding this comment.
@tmilnthorp Pushed new commit with a new ToDeltaUnit() method.
Use ToString() and Enum.Parse() to get slightly more robust conversion without having to generate a switch statement for all cases.
|
Self-merging this now just to get a beta nuget out. @tmilnthorp Please let me know if you have some reservations on my latest change. |
angularsen
added a commit
that referenced
this pull request
Nov 16, 2018
This reverts commit b0361ba. Per discussion in: #560 (comment) We can't justify this change just yet, we need to know if the majority of our userbase expects this new Temperature arithmetic or the old one.
Closed
Closed
angularsen
added a commit
that referenced
this pull request
Dec 16, 2018
# 4.0.0 Release This PR will serve as the list of items to complete and it will be updated to show the progress before finally merged into `master` when completed. We all have busy schedules, so **if you want to help move this work forward then that is much appreciated!** ## The main theme is to reduce binary size In two years it has grown from 280 kB to 1.4 MB and a lot of it is due to unnecessary syntactic sugar with many method overloads for various number types and nullable types - for every of our 800+ units! It simply adds up to a big total. These items are chosen from #180, trying to include as many of the low hanging fruits as possible, while still keeping the list short and realistic to complete in a reasonably short time - we are all busy in our daily lives. We can always have more major version bumps later than trying to perfect it all now. ## Feature complete before November, merged before mid-December I would like to aim for a "beta" pre-release nuget sometime in October with all the items completed, so we can have some time to test it before releasing the final, stable 4.0.0 version before Christmas holidays. We should have the work items list more or less final before Monday, October 8th. ## Changes #### Added - [x] UnitSystem with a different meaning, now defines the base units for a unit system (#524) #### Removed - [x] Replace netstandard1.0 target with netstandard2.0, to avoid the extra dependencies (#477) - [x] Remove code marked as `[Obsolete]`, such as `VolumeUnit.Teaspoon` (#490) - [x] Remove nullable `From` factory methods (#483) - [x] Remove extension methods on _nullable_ number types (#483) - [x] Remove all number extension methods (#497) - [x] Remove `Length2d`, replaced by `Area` (#501) - [x] Remove static methods on `UnitSystem`, should use `UnitSystem.Default` instead (#496) - [x] Remove unit parameter from `ToString()` methods (#546) #### Changed - [x] Throw exception on NaN values in static constructor methods, like `FromMeters()` (#502, see #176 (comment)) - [x] Throw if unit was not specified when constructing quantities, (#499 - #389 (comment)) - [x] Stricter parsing (#343 and #180 (comment)) - [x] Remove unit parameter from `ToString()` methods (#546) - [x] Change Temperature arithmetic back to use base unit Kelvin (#550, see #518) #### Renamed - [x] Correct SingularName for some Flow unit definitions (#494, see #360) - [x] Split/rename UnitSystem into UnitParser, GlobalConfiguration, UnitAbbreviationsCache (#511) #### Fixed - [x] Search for any TODO comments in the code to address, remove comment and either fix or create issue (5d24432) - [x] Update README with v4 changes #498 - [x] Do not try/catch in UnitConverter.Try-methods (#506) - [x] Do not try/catch in `Length.TryParse()` and for other quantities (#507) - [x] Add/update changelog in wiki for v4 with some summary copied from this issue, some v3 to v4 migration instructions and briefly explain why some stuff were removed ## Milestones - [x] Finalize work items list (Monday, October 8) - [x] Release 4.0.0-beta1, feature complete (Wednesday, October 31) - [ ] ~Show release notes and upgrade guide when upgrading to 4.x nuget, if possible~ - [x] Release 4.0.0 (Monday, December 17)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #518
TemperatureDelta)Multiply()andDivide()methods-operator overloadTemperature.ToDelta(other)instance methodTemperatureDelta.FromTemperatures(left, right)static method