Proposal: use "enhanced enums" in the repo for reduced boilerplate

[nate-thegrate]

There are a few situations where the same enum logic is implemented over and over again.

A good example would be distinguishing an AnimationStatus between forward/complete and reverse/dismissed.

15 style guide violations (click to collapse)

flutter/dev/integration_tests/flutter_gallery/lib/demo/material/backdrop_demo.dart

Line 281 in 31847bc

return status == AnimationStatus.completed || status == AnimationStatus.forward;

flutter/dev/integration_tests/flutter_gallery/lib/demo/shrine/backdrop.dart

Line 235 in 31847bc

return status == AnimationStatus.completed || status == AnimationStatus.forward;

flutter/dev/integration_tests/flutter_gallery/lib/demo/shrine/expanding_bottom_sheet.dart

Line 236 in 31847bc

return status == AnimationStatus.completed || status == AnimationStatus.forward;

flutter/dev/integration_tests/flutter_gallery/lib/gallery/backdrop.dart

Line 260 in 31847bc

final bool isOpen = status == AnimationStatus.completed || status == AnimationStatus.forward;

flutter/dev/integration_tests/new_gallery/lib/pages/splash.dart

Lines 71 to 72 in 31847bc

	return _controller.status == AnimationStatus.completed ||
	_controller.status == AnimationStatus.forward;

flutter/dev/integration_tests/new_gallery/lib/studies/reply/adaptive_nav.dart

Lines 525 to 526 in 31847bc

	return status == AnimationStatus.completed ||
	status == AnimationStatus.forward;

flutter/dev/integration_tests/new_gallery/lib/studies/shrine/app.dart

Lines 122 to 123 in 31847bc

	if (status == AnimationStatus.completed ||
	status == AnimationStatus.forward) {

flutter/dev/integration_tests/new_gallery/lib/studies/shrine/backdrop.dart

Lines 214 to 215 in 31847bc

	return status == AnimationStatus.completed ||
	status == AnimationStatus.forward;

flutter/dev/integration_tests/new_gallery/lib/studies/shrine/expanding_bottom_sheet.dart

Lines 343 to 344 in 31847bc

	return status == AnimationStatus.completed ||
	status == AnimationStatus.forward;

flutter/packages/flutter/lib/src/material/navigation_bar.dart

Lines 1309 to 1310 in 31847bc

	return animation.status == AnimationStatus.forward
	|| animation.status == AnimationStatus.completed;

flutter/packages/flutter/lib/src/material/navigation_drawer.dart

Line 691 in 31847bc

return animation.status == AnimationStatus.forward || animation.status == AnimationStatus.completed;

flutter/packages/flutter/lib/src/material/scaffold.dart

Lines 610 to 611 in 31847bc

	_snackBarController!.status == AnimationStatus.forward ||
	_snackBarController!.status == AnimationStatus.completed,

flutter/packages/flutter/lib/src/material/scaffold.dart

Lines 3206 to 3207 in 31847bc

	widget.animationController.status == AnimationStatus.forward
	|| widget.animationController.status == AnimationStatus.completed,

flutter/packages/flutter/lib/src/widgets/animated_cross_fade.dart

Lines 320 to 321 in 31847bc

	final bool transitioningForwards = _controller.status == AnimationStatus.completed ||
	_controller.status == AnimationStatus.forward;

flutter/packages/flutter/lib/src/widgets/routes.dart

Lines 1939 to 1940 in 31847bc

	ignoring: animation!.status == AnimationStatus.reverse || // changedInternalState is called when animation.status updates
	animation!.status == AnimationStatus.dismissed, // dismissed is possible when doing a manual pop gesture

Rather than refactoring all of these into switch expressions, I think it'd be best to take advantage of enhanced enum features and improve AnimationStatus directly.

[FusoraTech]

You showed that currently have something like this:

enum AnimationStatus {
  /// The animation is stopped at the beginning.
  dismissed,

  /// The animation is running from beginning to end.
  forward,

  /// The animation is running backwards, from end to beginning.
  reverse,

  /// The animation is stopped at the end.
  completed,
}

final AnimationStatus status = _controller.status;

final bool returnValue = status == AnimationStatus.completed || status == AnimationStatus.forward; 

Would you mind to show the proposed cleaner, simpler and more performant preferred new way that reduces the boilerplate?

[nate-thegrate]

@FusoraTech I forgot to link to this issue in the pull request I made, my bad!

I was going to wait to refactor until that PR is approved, but I'd be happy to share some ideas now 🙂

animated_cross_fade.dart

before

bool get _isTransitioning => _controller.status == AnimationStatus.forward || _controller.status == AnimationStatus.reverse;

Widget build(BuildContext context) {
  final bool transitioningForwards = _controller.status == AnimationStatus.completed ||
                                     _controller.status == AnimationStatus.forward;
  if (transitioningForwards) {
    // ...
  }
}

• transitioningForwards is instantiated only to be used once
• style guide violations:
  • Avoid using == with enum values
  • Prefer a maximum line length of 80 characters

after

bool get _isTransitioning => _controller.isRunning;

Widget build(BuildContext context) {
  if (_controller.aimedForward) {
    // ...
  }
}

• since _controller.isRunning is so concise, we might be able to factor out the _isTransitioning getter entirely!

[FusoraTech]

I understand. The idea is to make specialized getters right inside those "smart enums", replacing complex reusable ifs with switches when possible, and reducing the boilerplate replacing those long ifs spread everywhere by those new concise getters.

[FusoraTech]

Just to be clear, my worry was about exchanging performance in one place by complexity in another place, and performance penalty adding an extra call layer, just to satisfy a code clarity guidance.