Skip to content

Turning if chains into shorter switch statements - #144977

Merged
auto-submit[bot] merged 16 commits into
flutter:masterfrom
nate-thegrate:shorter-switch-statements
Mar 13, 2024
Merged

auto-submit[bot] merged 16 commits into
flutter:masterfrom
nate-thegrate:shorter-switch-statements

Conversation

@nate-thegrate

@nate-thegrate nate-thegrate commented Mar 12, 2024 •

Copy link
Copy Markdown
Contributor

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, this one is full of switch statements that make things more concise!

@github-actions github-actions Bot added a: tests "flutter test", flutter_test, or one of our tests tool Affects the "flutter" command-line tool. See also t: labels. framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages f: scrolling Viewports, list views, slivers, etc. p: cupertino_ui cupertino_ui package in flutter/packages d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: gestures flutter/packages/flutter/gestures repository. f: focus Focus traversal, gaining or losing focus labels Mar 12, 2024

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM with a tiny formatting nit

setState(() {
_animationDirection = status;
});
setState(() => _animationDirection = status);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general, we prefer styling setState as a block like it was before. Can you revert this formatting change?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had a look through the repo and you're right—an overwhelming majority of setState calls use a function body even if there's just one statement.

I was surprised at how many were formatted as a single line without the => syntax:

setState(() { _value = newValue; });

I guess now I know to use curly braces in the Flutter repo from now on.

@goderbauer
goderbauer requested a review from loic-sharma March 12, 2024 16:28
@nate-thegrate
nate-thegrate marked this pull request as ready for review March 12, 2024 16:29
Comment thread packages/flutter/test/cupertino/nav_bar_transition_test.dart

@loic-sharma loic-sharma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the excellent contribution!

@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 12, 2024
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2024
nate-thegrate added a commit that referenced this pull request May 10, 2024
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]>
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
@nate-thegrate nate-thegrate added the refactor Improving readability/efficiency without behavioral changes label Jul 12, 2024
chunhtai pushed a commit to chunhtai/packages that referenced this pull request Jun 12, 2026
Previous "if-chains" pull requests:
- flutter/flutter#144905
- flutter/flutter#144977
- flutter/flutter#145194
- flutter/flutter#146293
- flutter/flutter#147472

<br>

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

fixes flutter/flutter#144903

---------

Co-authored-by: Victor Sanni <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: focus Focus traversal, gaining or losing focus f: gestures flutter/packages/flutter/gestures repository. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages p: material_ui material_ui package in flutter/packages refactor Improving readability/efficiency without behavioral changes tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants