Skip to content

Commit

Permalink
Intensive if chain refactoring (flutter#145194)
Browse files Browse the repository at this point in the history 
This pull request refactors if-statements into switch expressions, as part of the effort to solve issue flutter#144903.

Making changes beyond just swapping syntax is more difficult (and also more difficult to review, I apologize), but much more satisfying too.
  • Loading branch information
@nate-thegrate
nate-thegrate committed Mar 22, 2024
1 parent 859eb2e commit 7fb35db
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 352 deletions.
42 changes: 14 additions & 28 deletions dev/benchmarks/macrobenchmarks/lib/src/very_long_picture_scrolling.dart
View file
Expand Up @@ -97,7 +97,7 @@ class PaintTest extends CustomPainter {

@override
void paint(Canvas canvas, Size size) {
final double height = size.height;
final double halfHeight = size.height / 2;
double x = 0;
const double strokeSize = .5;
const double zoomFactor = .5;
Expand Down Expand Up @@ -125,19 +125,12 @@ class PaintTest extends CustomPainter {
final Float32List offsets = Float32List(consolidate ? waveData.length * 4 : 4);
int used = 0;
for (index = 0; index < waveData.length; index++) {
Paint curPaint;
Offset p1;
if (waveData[index].isNegative) {
curPaint = paintPos;
p1 = Offset(x, height * 1 / 2 - waveData[index] / 32768 * (height / 2));
} else if (waveData[index] == 0) {
curPaint = paintZero;
p1 = Offset(x, height * 1 / 2 + 1);
} else {
curPaint = (waveData[index] == 0) ? paintZero : paintNeg;
p1 = Offset(x, height * 1 / 2 - waveData[index] / 32767 * (height / 2));
}
final Offset p0 = Offset(x, height * 1 / 2);
final (Paint curPaint, Offset p1) = switch (waveData[index]) {
< 0 => (paintPos, Offset(x, halfHeight * (1 - waveData[index] / 32768))),
> 0 => (paintNeg, Offset(x, halfHeight * (1 - waveData[index] / 32767))),
_ => (paintZero, Offset(x, halfHeight + 1)),
};
final Offset p0 = Offset(x, halfHeight);
if (consolidate) {
if (listPaint != null && listPaint != curPaint) {
canvas.drawRawPoints(PointMode.lines, offsets.sublist(0, used), listPaint);
Expand Down Expand Up @@ -179,7 +172,7 @@ class PaintSomeTest extends CustomPainter {

@override
void paint(Canvas canvas, Size size) {
final double height = size.height;
final double halfHeight = size.height / 2;
double x = 0;
const double strokeSize = .5;
const double zoomFactor = .5;
Expand All @@ -203,19 +196,12 @@ class PaintSomeTest extends CustomPainter {
..style = PaintingStyle.stroke;

for (int index = from; index <= to; index++) {
Paint curPaint;
Offset p1;
if (waveData[index].isNegative) {
curPaint = paintPos;
p1 = Offset(x, height * 1 / 2 - waveData[index] / 32768 * (height / 2));
} else if (waveData[index] == 0) {
curPaint = paintZero;
p1 = Offset(x, height * 1 / 2 + 1);
} else {
curPaint = (waveData[index] == 0) ? paintZero : paintNeg;
p1 = Offset(x, height * 1 / 2 - waveData[index] / 32767 * (height / 2));
}
final Offset p0 = Offset(x, height * 1 / 2);
final (Paint curPaint, Offset p1) = switch (waveData[index]) {
< 0 => (paintPos, Offset(x, halfHeight * (1 - waveData[index] / 32768))),
> 0 => (paintNeg, Offset(x, halfHeight * (1 - waveData[index] / 32767))),
_ => (paintZero, Offset(x, halfHeight + 1)),
};
final Offset p0 = Offset(x, halfHeight);
canvas.drawLine(p0, p1, curPaint);
x += zoomFactor;
}
Expand Down
63 changes: 9 additions & 54 deletions dev/manual_tests/lib/star_border.dart
View file
Expand Up @@ -474,58 +474,13 @@ const Color lerpToColor = Colors.red;
const BorderSide lerpToBorder = BorderSide(width: 5, color: lerpToColor);

ShapeBorder? lerpBorder(StarBorder border, LerpTarget target, double t, {bool to = true}) {
switch (target) {
case LerpTarget.circle:
if (to) {
return border.lerpTo(const CircleBorder(side: lerpToBorder, eccentricity: 0.5), t);
} else {
return border.lerpFrom(const CircleBorder(side: lerpToBorder, eccentricity: 0.5), t);
}
case LerpTarget.roundedRect:
if (to) {
return border.lerpTo(
const RoundedRectangleBorder(
side: lerpToBorder,
borderRadius: BorderRadius.all(
Radius.circular(10),
),
),
t,
);
} else {
return border.lerpFrom(
const RoundedRectangleBorder(
side: lerpToBorder,
borderRadius: BorderRadius.all(
Radius.circular(10),
),
),
t,
);
}
case LerpTarget.rect:
if (to) {
return border.lerpTo(const RoundedRectangleBorder(side: lerpToBorder), t);
} else {
return border.lerpFrom(const RoundedRectangleBorder(side: lerpToBorder), t);
}
case LerpTarget.stadium:
if (to) {
return border.lerpTo(const StadiumBorder(side: lerpToBorder), t);
} else {
return border.lerpFrom(const StadiumBorder(side: lerpToBorder), t);
}
case LerpTarget.polygon:
if (to) {
return border.lerpTo(const StarBorder.polygon(side: lerpToBorder, sides: 4), t);
} else {
return border.lerpFrom(const StarBorder.polygon(side: lerpToBorder, sides: 4), t);
}
case LerpTarget.star:
if (to) {
return border.lerpTo(const StarBorder(side: lerpToBorder, innerRadiusRatio: .5), t);
} else {
return border.lerpFrom(const StarBorder(side: lerpToBorder, innerRadiusRatio: .5), t);
}
}
final OutlinedBorder targetBorder = switch (target) {
LerpTarget.circle => const CircleBorder(side: lerpToBorder, eccentricity: 0.5),
LerpTarget.rect => const RoundedRectangleBorder(side: lerpToBorder),
LerpTarget.stadium => const StadiumBorder(side: lerpToBorder),
LerpTarget.polygon => const StarBorder.polygon(side: lerpToBorder, sides: 4),
LerpTarget.star => const StarBorder(side: lerpToBorder, innerRadiusRatio: 0.5),
LerpTarget.roundedRect => RoundedRectangleBorder(side: lerpToBorder, borderRadius: BorderRadius.circular(10)),
};
return to ? border.lerpTo(targetBorder, t) : border.lerpFrom(targetBorder, t);
}
15 changes: 5 additions & 10 deletions packages/flutter/lib/src/material/data_table.dart
View file
Expand Up @@ -1080,16 +1080,11 @@ class DataTable extends StatelessWidget {
for (int dataColumnIndex = 0; dataColumnIndex < columns.length; dataColumnIndex += 1) {
final DataColumn column = columns[dataColumnIndex];

final double paddingStart;
if (dataColumnIndex == 0 && displayCheckboxColumn && checkboxHorizontalMargin != null) {
paddingStart = effectiveHorizontalMargin;
} else if (dataColumnIndex == 0 && displayCheckboxColumn) {
paddingStart = effectiveHorizontalMargin / 2.0;
} else if (dataColumnIndex == 0 && !displayCheckboxColumn) {
paddingStart = effectiveHorizontalMargin;
} else {
paddingStart = effectiveColumnSpacing / 2.0;
}
final double paddingStart = switch (dataColumnIndex) {
0 when displayCheckboxColumn && checkboxHorizontalMargin == null => effectiveHorizontalMargin / 2.0,
0 => effectiveHorizontalMargin,
_ => effectiveColumnSpacing / 2.0,
};

final double paddingEnd;
if (dataColumnIndex == columns.length - 1) {
Expand Down
143 changes: 24 additions & 119 deletions packages/flutter/lib/src/material/menu_anchor.dart
View file
Expand Up @@ -2427,130 +2427,35 @@ class _MenuDirectionalFocusAction extends DirectionalFocusAction {
return;
}
final bool buttonIsFocused = anchor.widget.childFocusNode?.hasPrimaryFocus ?? false;
Axis orientation;
if (buttonIsFocused && anchor._parent != null) {
orientation = anchor._parent!._orientation;
} else {
orientation = anchor._orientation;
}
final Axis? parentOrientation = anchor._parent?._orientation;
final Axis orientation = (buttonIsFocused ? parentOrientation : null) ?? anchor._orientation;
final bool differentParent = orientation != parentOrientation;
final bool firstItemIsFocused = anchor._firstItemFocusNode?.hasPrimaryFocus ?? false;
final bool rtl = switch (Directionality.of(context)) {
TextDirection.rtl => true,
TextDirection.ltr => false,
};

assert(_debugMenuInfo('In _MenuDirectionalFocusAction, current node is ${anchor.widget.childFocusNode?.debugLabel}, '
'button is${buttonIsFocused ? '' : ' not'} focused. Assuming ${orientation.name} orientation.'));

switch (intent.direction) {
case TraversalDirection.up:
switch (orientation) {
case Axis.horizontal:
if (_moveToParent(anchor)) {
return;
}
case Axis.vertical:
if (firstItemIsFocused) {
if (_moveToParent(anchor)) {
return;
}
}
if (_moveToPrevious(anchor)) {
return;
}
}
case TraversalDirection.down:
switch (orientation) {
case Axis.horizontal:
if (_moveToSubmenu(anchor)) {
return;
}
case Axis.vertical:
if (_moveToNext(anchor)) {
return;
}
}
case TraversalDirection.left:
switch (orientation) {
case Axis.horizontal:
switch (Directionality.of(context)) {
case TextDirection.rtl:
if (_moveToNext(anchor)) {
return;
}
case TextDirection.ltr:
if (_moveToPrevious(anchor)) {
return;
}
}
case Axis.vertical:
switch (Directionality.of(context)) {
case TextDirection.rtl:
if (buttonIsFocused) {
if (_moveToSubmenu(anchor)) {
return;
}
} else {
if (_moveToNextFocusableTopLevel(anchor)) {
return;
}
}
case TextDirection.ltr:
switch (anchor._parent?._orientation) {
case Axis.horizontal:
case null:
if (_moveToPreviousFocusableTopLevel(anchor)) {
return;
}
case Axis.vertical:
if (buttonIsFocused) {
if (_moveToPreviousFocusableTopLevel(anchor)) {
return;
}
} else {
if (_moveToParent(anchor)) {
return;
}
}
}
}
}
case TraversalDirection.right:
switch (orientation) {
case Axis.horizontal:
switch (Directionality.of(context)) {
case TextDirection.rtl:
if (_moveToPrevious(anchor)) {
return;
}
case TextDirection.ltr:
if (_moveToNext(anchor)) {
return;
}
}
case Axis.vertical:
switch (Directionality.of(context)) {
case TextDirection.rtl:
switch (anchor._parent?._orientation) {
case Axis.horizontal:
case null:
if (_moveToPreviousFocusableTopLevel(anchor)) {
return;
}
case Axis.vertical:
if (_moveToParent(anchor)) {
return;
}
}
case TextDirection.ltr:
if (buttonIsFocused) {
if (_moveToSubmenu(anchor)) {
return;
}
} else {
if (_moveToNextFocusableTopLevel(anchor)) {
return;
}
}
}
}
final bool Function(_MenuAnchorState) traversal = switch ((intent.direction, orientation)) {
(TraversalDirection.up, Axis.horizontal) => _moveToParent,
(TraversalDirection.up, Axis.vertical) => firstItemIsFocused ? _moveToParent: _moveToPrevious,
(TraversalDirection.down, Axis.horizontal) => _moveToSubmenu,
(TraversalDirection.down, Axis.vertical) => _moveToNext,
(TraversalDirection.left, Axis.horizontal) => rtl ? _moveToNext : _moveToPrevious,
(TraversalDirection.right, Axis.horizontal) => rtl ? _moveToPrevious : _moveToNext,
(TraversalDirection.left, Axis.vertical) when rtl => buttonIsFocused ? _moveToSubmenu : _moveToNextFocusableTopLevel,
(TraversalDirection.left, Axis.vertical) when differentParent => _moveToPreviousFocusableTopLevel,
(TraversalDirection.left, Axis.vertical) => buttonIsFocused ? _moveToPreviousFocusableTopLevel : _moveToParent,
(TraversalDirection.right, Axis.vertical) when !rtl => buttonIsFocused ? _moveToSubmenu : _moveToNextFocusableTopLevel,
(TraversalDirection.right, Axis.vertical) when differentParent => _moveToPreviousFocusableTopLevel,
(TraversalDirection.right, Axis.vertical) => buttonIsFocused ? _moveToPreviousFocusableTopLevel : _moveToParent,
};
if (!traversal(anchor)) {
super.invoke(intent);
}
super.invoke(intent);
}

bool _moveToNext(_MenuAnchorState currentMenu) {
Expand Down
45 changes: 14 additions & 31 deletions packages/flutter/lib/src/material/reorderable_list.dart
View file
Expand Up @@ -393,39 +393,22 @@ class _ReorderableListViewState extends State<ReorderableListView> {
// If there is a header or footer we can't just apply the padding to the list,
// so we break it up into padding for the header, footer and padding for the list.
final EdgeInsets padding = widget.padding ?? EdgeInsets.zero;
late final EdgeInsets headerPadding;
late final EdgeInsets footerPadding;
late final EdgeInsets listPadding;

if (widget.header == null && widget.footer == null) {
headerPadding = EdgeInsets.zero;
footerPadding = EdgeInsets.zero;
listPadding = padding;
} else if (widget.header != null || widget.footer != null) {
switch (widget.scrollDirection) {
case Axis.horizontal:
if (widget.reverse) {
headerPadding = EdgeInsets.fromLTRB(0, padding.top, padding.right, padding.bottom);
listPadding = EdgeInsets.fromLTRB(widget.footer != null ? 0 : padding.left, padding.top, widget.header != null ? 0 : padding.right, padding.bottom);
footerPadding = EdgeInsets.fromLTRB(padding.left, padding.top, 0, padding.bottom);
} else {
headerPadding = EdgeInsets.fromLTRB(padding.left, padding.top, 0, padding.bottom);
listPadding = EdgeInsets.fromLTRB(widget.header != null ? 0 : padding.left, padding.top, widget.footer != null ? 0 : padding.right, padding.bottom);
footerPadding = EdgeInsets.fromLTRB(0, padding.top, padding.right, padding.bottom);
}
case Axis.vertical:
if (widget.reverse) {
headerPadding = EdgeInsets.fromLTRB(padding.left, 0, padding.right, padding.bottom);
listPadding = EdgeInsets.fromLTRB(padding.left, widget.footer != null ? 0 : padding.top, padding.right, widget.header != null ? 0 : padding.bottom);
footerPadding = EdgeInsets.fromLTRB(padding.left, padding.top, padding.right, 0);
} else {
headerPadding = EdgeInsets.fromLTRB(padding.left, padding.top, padding.right, 0);
listPadding = EdgeInsets.fromLTRB(padding.left, widget.header != null ? 0 : padding.top, padding.right, widget.footer != null ? 0 : padding.bottom);
footerPadding = EdgeInsets.fromLTRB(padding.left, 0, padding.right, padding.bottom);
}
}
double? start = widget.header == null ? null : 0.0;
double? end = widget.footer == null ? null : 0.0;
if (widget.reverse) {
(start, end) = (end, start);
}

final EdgeInsets startPadding, endPadding, listPadding;
(startPadding, endPadding, listPadding) = switch (widget.scrollDirection) {
Axis.horizontal || Axis.vertical when (start ?? end) == null => (EdgeInsets.zero, EdgeInsets.zero, padding),
Axis.horizontal => (padding.copyWith(left: 0), padding.copyWith(right: 0), padding.copyWith(left: start, right: end)),
Axis.vertical => (padding.copyWith(top: 0), padding.copyWith(bottom: 0), padding.copyWith(top: start, bottom: end)),
};
final (EdgeInsets headerPadding, EdgeInsets footerPadding) = widget.reverse
? (startPadding, endPadding)
: (endPadding, startPadding);

return CustomScrollView(
scrollDirection: widget.scrollDirection,
reverse: widget.reverse,
Expand Down

0 comments on commit 7fb35db

Please sign in to comment.