andreinitescu / IconFont2Code

Generate C# class with constant fields for the icons in your font
277 stars 37 forks source link

[Enhancement] Generate Enum #13

Closed dimonovdd closed 3 years ago

dimonovdd commented 3 years ago

If we could get the class with constant fields and the enum with your plugin, then the two warring parties would be happy

andreinitescu commented 3 years ago

Can I use an Enum without having some sort of converter, which converts the unicode number to a string?

dimonovdd commented 3 years ago

I have a small project that uses enum for iconFonts. It would be cool to get something like this at the output. Icon.cs

Maybe I could even create some kind of draft PR, but I'm not good with JS and Html

dimonovdd commented 3 years ago

But you are right, we will always use Convert.ToChar.

string IconToString(object obj) => obj is Enum val ? $"{Convert.ToChar(val)}" : null;

IconHelper.cs

andreinitescu commented 3 years ago

That's what I thought. That call adds a small performance penalty. Do you think it's worth to have a perf penalty instead of just using a class field?

andreinitescu commented 3 years ago

What benefits does an enum brings, beside being "nicer" (which is debatable anyway)

dimonovdd commented 3 years ago

I probably like enumerations because they protect me. I know for sure that nothing but the allowed values will be used. With modern devices, this will not cause performance problems, but it will save we from bad developers.

andreinitescu commented 3 years ago

I probably like enumerations because they protect me. I know for sure that nothing but the allowed values will be used.

Unless I'm missing something, safety applies to class fields too: <Label Text="{x:Static MyIcons.Smile}" /> If you misname the public field, it will give a compilation error.

The biggest issue I see is this requires extra code, which many people will not be aware of. If I offer an option in the tool to produce code with enums too, it could produce a confusion about how to use enums. I'd need to think about ways how to explain that. More problems. Or maybe you have some ideas.

andreinitescu commented 3 years ago

I don't see any advantage in using enum over class fields, especially since using an enum requires more code.

dimonovdd commented 3 years ago

if Text property is not the enum, but a string, then we can set it to any value. And we'll have to use extra code to check it and throw exception.

Unfortunately, even if a documentation or a summary says that a method never returns null, I check that a result really has value. And in this case, I will also have to do checks.

<Label Text="MyIcons.Smile" />
<Label Text="Smile" />
<Label Text="\ue007" />
<Label Text="ue007" />
<Label Text="007" />
andreinitescu commented 3 years ago

That's not how the class fields should be used. I always use the x:Static markup extension:

<Label Text="{x:Static MyIcons.Smile}" />

Like I mentioned above, if you use the wrong field name, the app won't compile.

dimonovdd commented 3 years ago

This is just an example of what I encountered in someone else's code. But cases can be more complicated when we use bindings, converters and etc. If I use enum, I know for sure that the value will be correct, and I don't need any extra checks

andreinitescu commented 3 years ago

If I use enum, I know for sure that the value will be correct, and I don't need any extra checks

I can't see any example how class fields are worse than enums by avoiding mistakes.

dimonovdd commented 3 years ago

In this case, We can set absolutely any value. Nothing limit us to use only class constants. I really will have to subscribe to PropertyChanged event and check the value of Text every time

<Label Text="{x:Static MyIcons.Smile}" />

If Icon property is of type enum, then the set of possible values is limited

<icons:IconSource Icon="Smile" />

So it's unlikely I'll get a performance loss. Yes, I may be paranoid. But I really try to protect my code, as much as possible.

Everyone should choose what they prefer

andreinitescu commented 3 years ago

In this case, We can set absolutely any value. Nothing limit us to use only class constants.

Well, following your example, a developer could use a Label instead of your IconSource control.

In my opinion just Label with {x:Static} is a simple and effective solution.

I'm sorry but I'm not going to implement this. If you think you need this you can fork the project and implement it yourself, it's very easy. You just need to change from generating the code from "class" to "enum". You can reuse the logic to make sure the enum names are unique.

andreinitescu commented 3 years ago

Here's the two parts where you would need to just replace the code with the C# class and fields code with an C# enum: https://github.com/andreinitescu/IconFont2Code/blob/master/js/CSharpCodeGen/CSharpCodeGen.js#L17 https://github.com/andreinitescu/IconFont2Code/blob/master/js/CSharpCodeGen/CSharpCodeGen.js#L30 The field array is an array of items with name and value properties.