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

Code cleaning: using enum where possible #17352

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Arthur-Milchior
Copy link
Member

@Arthur-Milchior Arthur-Milchior commented Nov 4, 2024

This PR (quite simplified compared to first version), replace some else -> by an exhaustive constant list. This will ensure that if more enum are added, we won't forget to consider all enums

@david-allison
Copy link
Member

david-allison commented Nov 12, 2024

Could you split this into a few smaller PRs (~2/300 lines each)? This isn't going to be easy to review otherwise, especially as it's low priority.

The first commit in the series uses single variable names for a class (.B implies boolean), which doesn't feel at all intuitive. I suspect the rest is mergeable, but it's a huge time commitment in this state, and attracts conflicts


IntFlags existed because in the early days, Android was extremely memory constrained, and Int used less memory than enum. A very quick Google returned: https://medium.com/trade-me/android-then-and-now-intro-intdef-enums-bca22d5cca56, but you'll easily find better articles.

I believe it was an AOSP standard for many years.

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Nov 12, 2024
@Arthur-Milchior
Copy link
Member Author

@david-allison done

@david-allison
Copy link
Member

Tests fail on this one

When `when` is used on an enum, the `else ->` can always be
transformed into a complete enumeration of the enum's entries.

This ensure that each time a new entry is added, there is a warning
and the developer must decide whether or not a change is needed. While
the `else` may hide some required change.

So, for the sake of future developement, I removed `else ->` when it
makes sense.

I should note that I kept some `else ->` when the `when` consider only
a very small part of all possible enum value, in order to reduce the
size of the code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Conflicts Needs Author Reply Waiting for a reply from the original author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants