shpaass / yafc-ce

Powerful Factorio calculator/analyser that works with mods
GNU General Public License v3.0
52 stars 18 forks source link

[Todo] Refactor enums to not use ordinal comparison #269

Open shpaass opened 1 week ago

shpaass commented 1 week ago

Here's an abstract example of the current situation:

enum Foo {
Apple,
Bees,
Thing
}
[...]
    if (value >= Foo.Bees)

The if breaks if we add a value to enum that we did not intended to get into this if. It also breaks if we reorder the enum. Therefore, it's better to refactor such occasions into explicit instructions instead of the ordinal ones. For instance:

    if (value == Foo.Bees || value == Foo.Thing)

What do you guys think? Are there better ways to refactor it?

veger commented 1 week ago

I guess it is related to color 'encoding', which also depends on the other (and 'completeness') of the enum entries (see #104)

It is a quick way of doing things, but it is hacky indeed, as these entries do not have some ordering/relation. As long as it is a single person project, who hopefully remembers these 'agreements', it is fine. But I agree that something alternative is better.

Instead of adding

if (value == Foo.Bees || value == Foo.Thing)

I would wrap it in a function (of the enum), so it is even easier to add new entries, without finding all places where this was added.

if (Foo.IsNotAFruit(value)
DaleStan commented 1 week ago

I would wrap it in a function (of the enum),

I like this in theory, but enums can't have methods in C#. Shadow made this appear to work by declaring ButtonEvent as a struct, but that's a lot of boilerplate for a normal enum, and even more for a [Flags] enum.

shpaass commented 1 week ago

Googling "c# enum methods" returns a suggestion from MSDN to use extension methods.

What do you guys think about this approach? I think this way Foo.IsNotAFruit(value) becomes feasible. Rather enumVarName.IsNotAFruit().

veger commented 6 days ago

It took a while to understand this magic, but I think it was mostly due to the naming and formatting (they could have named the class GradesExtensions, now I was thinking that the magic was with the namespace).

I think this looks nice to use (with proper naming ofc :wink:)