refactor: replace ARGS_ with ARG_, standardize requireParcelable usage#21366
Open
ericli3690 wants to merge 2 commits into
Open
refactor: replace ARGS_ with ARG_, standardize requireParcelable usage#21366ericli3690 wants to merge 2 commits into
ericli3690 wants to merge 2 commits into
Conversation
Per ankidroid#19896, argument constants should begin with ARG_. Many files, including some I've created, use ARGS_. This commit fixes that. Assisted-by: Composer 2.5
There are useful extensions in BundleUtils courtesy of David (thanks David!) for accessing stored values in argument Bundles. Using them throughout the codebase significantly cleans up several call sites. I also believe it is better to use `lazy` to get stored arguments rather than defining a `get() =` to avoid re-computation of something that never changes.
5 tasks
david-allison
approved these changes
Jul 12, 2026
| ) : ViewModel() { | ||
| val args: ImageOcclusionArgs = | ||
| checkNotNull(savedStateHandle[IO_ARGS_KEY]) { "$IO_ARGS_KEY required" } | ||
| checkNotNull(savedStateHandle[ARG_IMAGE_OCCLUSION]) { "$ARG_IMAGE_OCCLUSION required" } |
Member
There was a problem hiding this comment.
This could have an extension to be cleaner
Member
|
Merge at your discretion, might warrant a second Reviewer as I'm unsure of the test coverage here |
david-allison
added
Needs Second Approval
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Assisted-by: Composer 2.5 (for ARG_ renaming grunt work)
Purpose / Description
Per the naming conventions we've decided on, argument constant names should begin with ARG_, not ARGS_. This change standardizes instances of ARGS_ I found in the codebase. I initially tried applying the change to all argument constants in the entire repo, but as you can guess that spiralled out of control really fast. To keep the scope limited and safe, I've just looked for all instances of "ARGS_", plus any files I've touched, and fixed those.
Also, at #19896, David suggested using
requireParcelablemore. I find the utility functions in BundleUtils very useful and have applied them everywhere possible after looking for instances ofrequireNotNullin the codebase. This includes two review reminder instances.Fixes
Approach
Renaming PR, followed by the standardization of requireParcelable, requireString, etc.
There were some
requireNotNullinstances that grabbed things like string arrays and long arrays from Bundles: no such utility function to handle these cases exists yet in BundleUtils so I just left those as is. Could be something to return to in the future, but this is very low priority.I also believe it is better to use
lazyto get stored arguments rather than defining aget() =to avoid re-computation of something that never changes.Overall, after applying
requireParcelableandlazy, I personally think a lot of argument usages are now cleaner.How Has This Been Tested?
Checklist