Replacing the ToString overloads with extensions#1552
Replacing the ToString overloads with extensions#1552lipchev wants to merge 1 commit intoangularsen:masterfrom
Conversation
|
Ok, here is the size difference:
Since we've only got one generic parameter, the bug with Resharper does not apply. The only breaking parts concern the cases where a given class doesn't have the I'll make separate PR, with the other version which simply removes the overloads and makes the parameters optional. |
|
Yeah, I don't like how default parameters for ToString() make it awkward to only pass a culture. It's just so common to have relevant ToString() overloads for this. Extension methods could work though, but it does have some potential developer expirence friction as you point out regarding missing namespace imports. I expect this to be fairly uncommon though and it only affects 2 of 4 ToString() overloads, and IDEs may do a decent job of importing namespaces for you. I guess it comes down to, is the 25 kB binary size reduction worth a potentially worse developer experience? ToString() is used a lot. I'm kinda leaning towards maybe not enough impact to justify this change, what do you think? |
|
With the perspective that we want to try and use extension methods more in general to avoid duplicating code for N quantities, maybe we'll just have to try this and see how it feels. |
|
For me personally it's just the interface method that's bothering me.. It might look like I'm optimizing for size, but that's really not a much of priority (in my use case). |
|
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
This PR was automatically closed due to inactivity. |
|
Merged as part of #1587 |
QuantityExtensions(in the Extensions folder) with the first two extensions for theToStringoverloadsToString(IFormatProvider?)overload from theIQuantityinterfaceToStringoverloads from the generated quantities (andHowMuch)