Miha-x64 / Mikes_IDEA_extensions

IntelliJ IDEA: missing parts.
Apache License 2.0
34 stars 7 forks source link

Request: add inspection of reaching SomeEnum.values()[someIndex] #40

Open AndroidDeveloperLB opened 1 year ago

AndroidDeveloperLB commented 1 year ago

This is for both Java and Kotlin.

Such a code can lead to bugs easily in case the order of the enum values change, or items get removed/replaced.

Miha-x64 commented 1 year ago

But wait, SomeEnum.valueOf(someName) can also lead to bugs if items get removed or renamed. Is there any way out?

AndroidDeveloperLB commented 1 year ago

@Miha-x64 Not the same. Basing on index is more problematic, as even adding new items could ruin it easily.

What you probably wanted to compare to is ordinal (which returns the index), and then you are indeed correct, because we are talking about relying on index, and indeed there is a warning on JavaDocs about it:

Most programmers will have no use for this method. It is designed for use by sophisticated enum-based data structures, such as java.util.EnumSet and java.util.EnumMap.

https://docs.oracle.com/javase/8/docs/api/java/lang/Enum.html

So an equivalent of SomeEnum.values()[someIndex] can't be SomeEnum.valueOf(someName). It would be something like this:

SomeEnum.values().forEach { 
    if(it.ordinal==someIndex)
        return it
}
throw IndexOutOfBoundsException()

It's written in more places that you shouldn't rely on index, hence you shouldn't use these. The order of items on the enum class is indeed by declaration, but it's not explicit. That's why people often suggest to add a field inside.

The "valueOf" is much more common than both of these, even combined. It's much more valid use than being based on index.

Have the inspection turned off by default, if you wish, but I still think it's useful.

Miha-x64 commented 1 year ago

The order of items on the enum class is indeed by declaration, but it's not explicit.

The funny thing is that enum names, actually, are also by declaration and not explicit.

Moving breaks order, renaming breaks names. Both without changing any integer/string literals.

AndroidDeveloperLB commented 1 year ago

You have a point, but renaming an item is like removing the old one and adding a different one instead. The Enum values are the ID of them. Not their order. When you create a new Enum value, you create a new instance of it that is different than the rest. Sure it might look identical to what it was before, but in terms of Enum, it's more like a new item, while removing the previous one.

Plus, it is much more different than just changing the order, and it's much easier to break, as you can add items in between, too, and remove items in between, too. That's more cases that it can break.

I mean that you are comparing 1 case (renaming) vs 3 (change order, put item, remove item). Renaming is much more obvious, too, as you have a different instance.