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.

Sign up for GitHub

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

Jump to bottom

refactor if chains and == with enum values #144903

Closed
nate-thegrate opened this issue Mar 10, 2024 · 0 comments · Fixed by #147793
Closed

refactor if chains and == with enum values #144903

nate-thegrate opened this issue Mar 10, 2024 · 0 comments · Fixed by #147793
Assignees
@nate-thegrate
Labels
c: proposal A detailed proposal for a change to Flutter framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project r: fixed Issue is closed as already fixed in a newer version team-framework Owned by Framework team triaged-framework Triaged by Framework team

Comments

@nate-thegrate
Copy link
Member

From the Flutter repo style guide:

Avoid using if chains or ?: or == with enum values


Example (toggle_buttons.dart):

// using "=="
bool _isFirstButton(int index, int length, TextDirection textDirection) {
  return index == 0 && ((direction == Axis.horizontal && textDirection == TextDirection.ltr) ||
    (direction == Axis.vertical && verticalDirection == VerticalDirection.down))
    || index == length - 1 && ((direction == Axis.horizontal && textDirection == TextDirection.rtl) ||
    (direction == Axis.vertical && verticalDirection == VerticalDirection.up));
}


// using a switch statement
bool _isFirstButton(int index, int length, TextDirection textDirection) {
  switch (direction) {
    case Axis.horizontal:
      return switch (textDirection) {
        TextDirection.rtl => index == length - 1,
        TextDirection.ltr => index == 0,
      };
    case Axis.vertical:
      return switch (verticalDirection) {
        VerticalDirection.up   => index == length - 1,
        VerticalDirection.down => index == 0,
      };
  }
}

Performing this refactor would increase the total line length, but in my opinion the improved readability is worth it.
@nate-thegrate nate-thegrate mentioned this issue Mar 10, 2024
Merged
@danagbemava-nc danagbemava-nc added in triage Presently being triaged by the triage team framework flutter/packages/flutter repository. See also f: labels. c: proposal A detailed proposal for a change to Flutter team-framework Owned by Framework team and removed in triage Presently being triaged by the triage team labels Mar 11, 2024
@danagbemava-nc danagbemava-nc changed the title if chains and == with enum values refactor if chains and == with enum values Mar 11, 2024
auto-submit bot pushed a commit that referenced this issue Mar 11, 2024
@nate-thegrate
Refactoring if chains into switch statements (#144905)
26e379e 
Based on issue #144903, this PR aims to bring the codebase more in line with the [Flutter repo style guide](https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-using-if-chains-or--or--with-enum-values):

> #### Avoid using `if` chains or `?:` or `==` with enum values

<br>

This change unfortunately increases the total line length, but it also improves readability.
@nate-thegrate nate-thegrate mentioned this issue Mar 12, 2024
Merged
@goderbauer goderbauer added P3 Issues that are less important to the Flutter project triaged-framework Triaged by Framework team labels Mar 12, 2024
auto-submit bot pushed a commit that referenced this issue Mar 13, 2024
@nate-thegrate
Turning if chains into shorter switch statements (#144977)
b9e5373 
This pull request is part of the effort to solve issue #144903.

In the past, my efforts to reduce line length involved refactoring away from switch statements, but unlike [yesterday's PR](#144905), this one is full of switch statements that make things more concise!
@nate-thegrate nate-thegrate mentioned this issue Mar 15, 2024
Merged
auto-submit bot pushed a commit that referenced this issue Mar 22, 2024
@nate-thegrate
Intensive if chain refactoring (#145194)
7fb35db 
This pull request refactors if-statements into switch expressions, as part of the effort to solve issue #144903.

Making changes beyond just swapping syntax is more difficult (and also more difficult to review, I apologize), but much more satisfying too.
@nate-thegrate nate-thegrate mentioned this issue Apr 4, 2024
Merged
auto-submit bot pushed a commit that referenced this issue Apr 23, 2024
@nate-thegrate
flutter/lib/src/: refactoring if-chains into switch expressions (#1…
a83e111 
…46293)

Based on issue #144903, this pull request aims to bring the codebase more in line with the [Flutter repo style guide](https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-using-if-chains-or--or--with-enum-values):

> ### Avoid using `if` chains or `?:` or `==` with enum values
This was referenced Apr 27, 2024
Merged
Merged
nate-thegrate added a commit that referenced this issue May 10, 2024
@nate-thegrate @victorsanni
if chains → switch expressions (#147793)
4734d80 
Previous "if-chains" pull requests:
- #144905
- #144977
- #145194
- #146293
- #147472

<br>

I think this one should be enough to wrap things up!

fixes #144903

---------

Co-authored-by: Victor Sanni <[email protected]>
@danagbemava-nc danagbemava-nc added the r: fixed Issue is closed as already fixed in a newer version label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nate-thegrate nate-thegrate

Labels
c: proposal A detailed proposal for a change to Flutter framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project r: fixed Issue is closed as already fixed in a newer version team-framework Owned by Framework team triaged-framework Triaged by Framework team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

if chains → switch expressions nate-thegrate/flutter
3 participants
@goderbauer @nate-thegrate @danagbemava-nc