Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
AndroidDeveloperLB opened this issue Nov 20, 2022 · 4 comments

Comments

@AndroidDeveloperLB
Copy link

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
Copy link
Owner

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

@AndroidDeveloperLB AndroidDeveloperLB changed the title Request: add inspection of reaching someEnum.values()[someIndex] Request: add inspection of reaching SomeEnum.values()[someIndex] Nov 20, 2022
@AndroidDeveloperLB
Copy link
Author

AndroidDeveloperLB commented Nov 20, 2022

@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
Copy link
Owner

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
Copy link
Author

AndroidDeveloperLB commented Dec 16, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants