Skip to content

Refactor: remove isAvailable(), make build() return Skill<*>?#415

Draft
mretallack wants to merge 1 commit into
DicioTeam:mainfrom
mretallack:refactor/remove-isAvailable-race-condition
Draft

Refactor: remove isAvailable(), make build() return Skill<*>?#415
mretallack wants to merge 1 commit into
DicioTeam:mainfrom
mretallack:refactor/remove-isAvailable-race-condition

Conversation

@mretallack
Copy link
Copy Markdown

Eliminates the race condition where a skill could be deemed available under one locale but then built/executed under a different locale after a language change.

By merging the availability check into build() and returning null when the skill cannot be constructed, there is no longer a window between checking availability and building the skill where state can change.

  • Remove SkillInfo.isAvailable() abstract method
  • Change SkillInfo.build() return type from Skill<> to Skill<>?
  • Update all 15 SkillInfo implementations to return null if unavailable
  • Update SkillHandler to use mapNotNull for building skills
  • Update SkillSettingsScreen to use build() != null
  • Update test mocks and preview providers

Addresses the root cause of the NPE in #412 at the architecture level.

Eliminates the race condition where a skill could be deemed available
under one locale but then built/executed under a different locale after
a language change.

By merging the availability check into build() and returning null when
the skill cannot be constructed, there is no longer a window between
checking availability and building the skill where state can change.

- Remove SkillInfo.isAvailable() abstract method
- Change SkillInfo.build() return type from Skill<*> to Skill<*>?
- Update all 15 SkillInfo implementations to return null if unavailable
- Update SkillHandler to use mapNotNull for building skills
- Update SkillSettingsScreen to use build() != null
- Update test mocks and preview providers

Addresses the root cause of the NPE in DicioTeam#412 at the architecture level.
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.

1 participant