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: Avoid nested conditional expressions whenever possible [DCM] #2784

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

luanpotter
Copy link
Member

Description

I was playing around with the rule avoid-nested-conditional-expressions.
While I philosophically agree with the rule, sadly Dart's new switch-as-an-expression, while a much-needed improvement, still pales in comparison to Kotlin's powerful when, which makes it impossible to convert all cases of nested ternaries into something actually better. In some cases, given the tools that Dart provides, I think nested ternary is the best option.
Notwithstanding, I used the rule to catch many cases where the new switch expression can be used to vastly simplify our codebase, making it much more readable.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

@luanpotter luanpotter force-pushed the luan.avoid-nested-conditional-expressions branch 2 times, most recently from 8820897 to daa1f83 Compare October 1, 2023 19:56
@luanpotter luanpotter force-pushed the luan.avoid-nested-conditional-expressions branch from daa1f83 to cbbc44f Compare October 1, 2023 20:08
@luanpotter luanpotter marked this pull request as ready for review October 1, 2023 20:32
: isString
? ExpressionType.string
: ExpressionType.unknown;
return switch (this) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this new way will be less performant if the getters will be used a lot when going through a deep tree for example, but it's way more readable. I guess it's not worth optimizing for performance here already though!

@spydon spydon merged commit 7b6a571 into main Oct 2, 2023
7 checks passed
@spydon spydon deleted the luan.avoid-nested-conditional-expressions branch October 2, 2023 09:12
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

2 participants