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

refactor: Break navigation dependency cycles with :core:navigation #305

Merged
merged 3 commits into from
Dec 7, 2023

Conversation

nikclayton
Copy link
Contributor

The previous code generally started an activity by having the activity provide a method in a companion object that returns the relevant intent, possibly taking additional parameters that will be included in the intent as extras.

E.g., if A wants to start B, B provides the method that returns the intent that starts B.

This introduces a dependency between A and B.

This is worse if B also wants to start A.

For example, if A is StatusListActivity and B isViewThreadActivity. The user might click a status in StatusListActivity to view the thread, starting ViewThreadActivity. But from the thread they might click a hashtag to view the list of statuses with that hashtag. Now StatusListActivity and ViewThreadActivity have a circular dependency.

Even if that doesn't happen the dependency means that any changes to B will trigger a rebuild of A, even if the changes to B are not relevant.

Break this dependency by adding a :core:navigation module with an app.pachli.core.navigation package that contains Intent subclasses that should be used instead. The quadrant plugin is used to generate constants that can be used to launch activities by name instead of by class, breaking the dependency chain.

The plugin uses the Activity names from the manifest, so when an activity is moved in the future the constant will automatically update to reflect the new package name.

If the activity's intent requires specific extras those are passed via the constructor, with companion object methods to extract them from the intent.

Using the intent classes from this package is enforced by a lint IntentDetector which will warn if any intents are created using a class literal.

The previous code generally started an activity by having the activity
provide a method in a companion object that returns the relevant intent,
possibly taking additional parameters that will be included in the
intent as extras.

E.g., if A wants to start B, B provides the method that returns the
intent that starts B.

This introduces a dependency between A and B.

This is worse if B also wants to start A.

For example, if A is `StatusListActivity` and B is`ViewThreadActivity`.
The user might click a status in `StatusListActivity` to view the
thread, starting `ViewThreadActivity`. But from the thread they might
click a hashtag to view the list of statuses with that hashtag. Now
`StatusListActivity` and `ViewThreadActivity` have a circular
dependency.

Even if that doesn't happen the dependency means that any changes to
B will trigger a rebuild of A, even if the changes to B are not
relevant.

Break this dependency by adding a `:core:navigation` module with an
`app.pachli.core.navigation` package that contains `Intent` subclasses
that should be used instead. The `quadrant` plugin is used to
generate constants that can be used to launch activities by name
instead of by class, breaking the dependency chain.

The plugin uses the `Activity` names from the manifest, so when an
activity is moved in the future the constant will automatically update
to reflect the new package name.

If the activity's intent requires specific extras those are passed via
the constructor, with companion object methods to extract them from the
intent.

Using the intent classes from this package is enforced by a lint
`IntentDetector` which will warn if any intents are created using a
class literal.
@nikclayton nikclayton merged commit 1214cf7 into pachli:main Dec 7, 2023
6 checks passed
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

Successfully merging this pull request may close these issues.

None yet

1 participant