Feature Request: Provide option to use "static readonly" instead of "const"

[ChristopherStephan]

As constants are processed at compile time, they are replaced with their values as soon as code is compiled. If you are using the mapping class in different versions across dlls this can lead to a wrong lookup and icons are mixed up in your app. Using read-only properties or static readonly fields can prevent this issue from happening.

[andreinitescu]

Because the const values are inlined during compilation, at runtime it run faster than other ways (static readonly or properties). For desktop apps, it doesn't matter, but it matters for mobile apps (Xamarin).

Did you actually go into a problem with the 'const' on the icon class? I am asking because I wonder how could someone generate a new class but without also recompile?

[ChristopherStephan]

We have a Xamarin app that is modularised with many nuget packages. We have a UI library as a nuget package that is referenced by multiple other nuget packages. The UI library contains a font file as an EmbeddedResource. When we add new icons to the icon font, these icons are added not necessarily "at the end" of the font but for example in the "middle" of the font. This means that unicode values in the mapping class change between updated and not updated version. If module A uses the UI library without the icon update and another module B uses the UI library with the icon update it can lead to a mix of icons in the UI that belongs to module A. The mapping class of module A has the "old" unicodes compiled in, which results in a wrong look up.

For example:

Before icon update: public const string Banana = "\uf223"; After icon update: public const string Banana = "\uf252"; After icon update: public const string Tomato = "\uf223";

One solution would be to have always the same version of the UI library across all client nuget packages, but this is not desired in our app.

I experimented with versioning the font file by renaming it, when it is updated. This did not work either.

Adapting the icon font generation in a way that icons are only added at the "end" is not desired either. Deleting icons in an icon font update should also be possible.

So we changed the mapping classes from "const" to "static readonly" to enable that we actually can have different UI library versions across modules. So far we did not encounter any noticeable performance issues.

BTW updating icon fonts is still buggy on Android: https://github.com/xamarin/Xamarin.Forms/issues/11843

[andreinitescu]

When we add new icons to the icon font, these icons are added not necessarily "at the end" of the font but for example in the "middle" of the font.

That's a key thing. That's definitely not going to work with 'const'. Thanks for describing your scenario.

[andreinitescu]

Do you think it's worth to actually implement this? It's a simple matter of doing a manual rename in VS from 'const' to 'static readonly'. It should take a minute. How often do you need to do this? Or are you thinking of making it more obvious somehow to avoid having to deal with the issue again due to the nature of the const compilation?

[ChristopherStephan]

Or are you thinking of making it more obvious somehow to avoid having to deal with the issue again due to the nature of the const compilation?

Yes. I just wanted to enforce that the developer needs to think about using const or static readonly. Maybe our scenario is too edge casy and it is really not worth the time implementing a choice between those as a feature. Maybe the advanced software engineer would have seen problems with usings const in our use case anyway.

[andreinitescu]

After some more thinking:

Only a new setting to choose between 'const' and 'static readonly' won't help to raise the awareness. Also, any new setting makes the settings panel more complicated and it increases the chance to be completely ignored.

What do you think about just adding a new class comment: https://github.com/andreinitescu/IconFont2Code/commit/3f987e455d31a396d716b25b5510d14b6544231a? I think it should work nicely. Please try it out, it's deployed.

[ChristopherStephan]

Nice!! Approved. 😄