Skip to content

Turning if chains into shorter switch statements #144977

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
Merged
auto-submit merged 16 commits into from
Mar 13, 2024
Merged

Turning if chains into shorter switch statements #144977

auto-submit merged 16 commits into from
Mar 13, 2024

Conversation

@nate-thegrate
Copy link
Contributor

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

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. f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. f: cupertino flutter/packages/flutter/cupertino repository 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 approved these changes Mar 12, 2024
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

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

packages/flutter/lib/src/material/navigation_bar.dart Outdated
setState(() {
_animationDirection = status;
});
setState(() => _animationDirection = status);
Copy link
Member

@goderbauer goderbauer Mar 12, 2024

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
Member

@goderbauer goderbauer Mar 12, 2024

Choose a reason for hiding this comment

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

same below.

Copy link
Contributor Author

@nate-thegrate nate-thegrate Mar 12, 2024

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
loic-sharma
loic-sharma approved these changes Mar 12, 2024
View reviewed changes
Copy link
Member

@loic-sharma loic-sharma left a comment

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
b9e5373 Turning if chains into shorter switch statements (flutter…
97d2890 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
e54fed2 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
c85ea53 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
c00c5cf 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
afe822a 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 14, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
5c044fb 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
6522ea0 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
5a3dc5e 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
1f032d5 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
76ed9d6 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
3ee1bdb 
…/flutter#144977)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
7b65baa 
…/flutter#144977)
@HansMuller HansMuller mentioned this pull request Mar 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
4c43e29 
…/flutter#144977)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
3fb85ea 
…/flutter#144977)
@nate-thegrate nate-thegrate mentioned this pull request May 3, 2024
Merged
nate-thegrate added a commit that referenced this pull request 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]>
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
@engine-flutter-autoroll
b9e5373 Turning if chains into shorter switch statements (flutter…
2f3a1fc 
…/flutter#144977)
@nate-thegrate nate-thegrate added the refactor Improving readability/efficiency without behavioral changes label Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@loic-sharma loic-sharma loic-sharma approved these changes

@goderbauer goderbauer goderbauer approved these changes

Assignees

No one assigned

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: cupertino flutter/packages/flutter/cupertino repository f: focus Focus traversal, gaining or losing focus f: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. refactor Improving readability/efficiency without behavioral changes tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nate-thegrate @loic-sharma @goderbauer