Added ability to change the default unit for .ToString() #133
Conversation
|
Don't feel any rush to merge this/release a new version. It's just a "nice to have" thing I created for fun. |
|
I think this is a fair change. I've had the same thought a couple of times how to let people choose their preferred unit in ToString(), but it was never a big enough concern for me to deal with it. Looks good to me, just fix a bit of whitspace and this should be ready to merge. |
|
I've replaced all tabs with white space now. |
|
Great! I'm unavailable this whole week but will get back to you. |
| } | ||
|
|
||
| [Test] | ||
| public void ToStringReturnsCorrectNumberAndUnitWithCentiliterAsDefualtUnit() |
There was a problem hiding this comment.
I like you went the whole 9 yards here, but I do think we should avoid adding tons of tests that basically test the same thing. I really just want to test the default behavior and one or two custom ToStringDefaultUnit values to ensure the implementation is working. Testing every unit seems to me to add little benefit since all units are generated the same way.
I propose you remove the powershell modification that generates these tests per unit, regenerate the code and instead manually write some test cases you find sufficient to verify the implementation.
Add cubic micrometer unit
|
Just noticed that there is a bug/design limitation regarding structs and |
|
Interesting link, never knew about that. |
|
It seems like the behavior changes depending on where you've put breakpoints in the code and if you step or run between said breakpoints. I've seen {2 cm}, {UnitsNet.Length} and {2 (no abbreviation for LengtUnit.Centimeter)} so far... |
|
The only way to work around it (that I can think of) would be to store a field in each struct with the unit to use. But that would increase the size of the struct and isn't something I would like to do. I just remove some tests and commit. It sounded like this is something that will be fixed in a later version of Visual Studio. |
|
Did you try |
|
Yes, it made Visual Studio lock up completely for a few seconds. |
…/github.com/eriove/UnitsNet into feature/changeable-default-unit-in-tostring
|
It looks like I failed with the rebase upstream (again)... |
|
Remember you can rebase your branch onto origin/master and force push to fix mistakes in your git commit history. |
|
This looks good to merge to me, is it done? |
|
It's done. |
…n-tostring Add ability to change default unit for ToString()
|
Awesome! Will release soon, waiting for another PR. |
|
Nuget 3.26 is now out. |
2 participants
For many measures the unit that makes most sense varies between applications. I added the static property
ToStringDefaultUnitwhich can be used to set the default unit to use for a ToString() without arguments. This primarily makes debugging easier. Existing behaviour will be kept as long asToStringDefaultUnitisn't set.The tests generated aren't thread safe but as far as I know NUnit doesn't support multi-threaded tests.
I imagined this property would be used by setting it once in a static constructor for a solution, or that it is set manually when someone starts debugging. This makes
ToString()even more dangerous than usual since the result may vary if some other part of the code may setToStringDefaultUnitbut since the same thing will happen if someone sets the current culture most developers should know that it's bad practice to not add input arguments toToString()... (Yes I see that bug at least once a month)