UnitsNet icon indicating copy to clipboard operation
UnitsNet copied to clipboard

Readonly structs

Open sonnemaf opened this issue 2 years ago • 3 comments

Should/could the structs in this project be made readonly. I think they should. All primitive struct types from Microsoft are readonly.

Readonly structs have a lot of advantages where there are no defensive copies for readonly fields, in parameters, 'readonly locals + returns`.

sonnemaf avatar Aug 31 '22 11:08 sonnemaf

As far as I see, everything compiles with public readonly partial struct as well.

Brunni avatar Sep 02 '22 19:09 Brunni

Sounds like a good improvement as long as it compiles for nestandard2.0. These structs were intended to be immutable already.

Would you be interested in attempting a pull request @sonnemaf ?

angularsen avatar Sep 02 '22 21:09 angularsen

Hi @angularsen

Good news but unfortunately I don't have time to implement it the next few weeks. Is it in a hurry? I will probably have time for it in october.

sonnemaf avatar Sep 07 '22 11:09 sonnemaf

No rush on my end :-)

angularsen avatar Oct 11 '22 07:10 angularsen

Hi,

I'm unable to clone the repository. I keep getting this error. Tried it on two computers.

image

I'm giving up. The change is really small. You only have to add the readonly text to the UnitsNet\CodeGen\Generators\UnitsNetGen\QuantityGenerator.cs on line 75, see screenshot below.

image

Can you do it yourself?

Regards,

Fons

sonnemaf avatar Oct 19 '22 13:10 sonnemaf

There is already a PR for this. https://github.com/angularsen/UnitsNet/pull/1135 Currently the maintainer @angularsen seems to be unavailable.

Brunni avatar Oct 31 '22 09:10 Brunni

@sonnemaf Sorry for the friction. You need to install Git LFS to clone the repo. https://git-lfs.github.com/

LFS is nice when it works, but GitHub charges for it when monthly bandwidth exceeds a certain amount. #1052 suggests downloading tooling binaries on-demand with an init script, instead of having it happen during checkout.

angularsen avatar Nov 02 '22 14:11 angularsen