Avoid these common Flutter mistakes with DCM
This article was originally posted in our blog.
We all make mistakes, it’s a part of our developer journey. But there are different tools that can help us in our way of creating great software that helps other people.
DCM (formerly Dart Code Metrics) is a great example of such a tool. It is a static analysis tool that detects the quality and consistency issues helping you reduce the number of bugs and deliver results faster to your users.
But before we begin, what does static analysis actually mean? Well, static analysis is the automated analysis of source code without executing the application. It does not know anything about how the code will be run, what actual data it will work with, etc. A good example of such tools is the Dart analyzer (you probably have it running right now, even if you are no aware of it) and any tool that is a linter.
How does it work? It would take a book (or maybe even two) to describe it in details, but in short, it runs a separate process that is able to parse your code into set of tokens, build AST and validate it based on some rules.
And yes, dynamic analysis also exists 🙂.
Let’s take a look at the various problems that you might encounter in your code and how DCM can help you find them together.
SetState
setState has an easy concept behind it (if you didn’t know, the name was inspired by the React frontend framework) — it simply notifies the framework that the internal state of this object has changed.
But there are several cases when you should use it carefully.
Avoiding asynchronous gap
In async functions, the state of a widget may have been disposed between await points, e.g. because the user moved to another screen, leading to errors calling setState. After each await point, i.e. when a Future is awaited, the possibility that the widget has been unmounted needs to be checked before calling setState.
For example,
class _MyWidgetState extends State<MyWidget> {
String message;
@override
Widget build(BuildContext context) {
return Button(
onPressed: () async {
String fromServer = await fetch(...);
setState(() {
message = fromServer;
});
},
child: Text(message),
);
}
}here fetch may complete after the widget was already disposed resulting in an exception. To avoid problems like that, after any async gap you need to ensure that the widget is still displayed on the screen. It can be done via the mounted property:
class _MyWidgetState extends State<MyWidget> {
String message;
@override
Widget build(BuildContext context) {
return Button(
onPressed: () async {
String fromServer = await fetch(...);
if (mounted) {
setState(() {
message = fromServer;
});
}
},
child: Text(message),
);
}
}this way you won’t get any exception. To ensure that practice is used, enable use-setstate-synchronously.
Avoiding inappropriate places of calling setState
Calling setState from initState, didUpdateWidget or build methods (or from any synchronous method called from them) is unnecessary and results in additional rerender (that you’d likely want to avoid).
For example,
class MyWidget extends StatefulWidget {
@override
_MyWidgetState createState() => _MyWidgetState();
}
class _MyWidgetState extends State<MyWidget> {
String myString = '';
@override
void initState() {
super.initState();
setState(() {
myString = "Hello";
});
if (condition) {
setState(() {
myString = "Hello";
});
}
myStateUpdateMethod();
}
@override
void didUpdateWidget(MyWidget oldWidget) {
setState(() {
myString = "Hello";
});
}
void myStateUpdateMethod() {
setState(() {
myString = "Hello";
});
}
@override
Widget build(BuildContext context) {
setState(() {
myString = "Hello";
});
if (condition) {
setState(() {
myString = "Hello";
});
}
myStateUpdateMethod();
return ElevatedButton(
onPressed: () => myStateUpdateMethod(),
onLongPress: () {
setState(() {
myString = data;
});
},
child: Text('PRESS'),
);
}
}there are seven places where setState call should be removed. Keeping unnecessary calls can negatively affect the performance of your app.
Use avoid-unnecessary-setstate to always be aware of any unnecessary calls.
Avoiding empty setState
Empty setState is simply considered an anti-pattern and you should always update the state inside the passed callback (or avoid calling setState, if there is no state to update). A rule to enforce that is called avoid-empty-setstate.
Render objects
Sometimes you end up in a situation when you need to create your own RenderObjects (if you never heard about them before, read this amazing article).
There are a least two problems you should be aware of: equality checks in setters and proper implementation for updateRenderObject.
Let’s start with the former:
class SomeRenderBox extends RenderBox {
double _dividerWidth;
double get dividerWidth => _dividerWidth;
set dividerWidth(double value) {
_dividerWidth = value;
markNeedsLayout();
}
}with RenderObjects it’s a common patterns to create a private variable and a pair of getter and setter (with a setter responsible for a markNeedsLayout call). But since the setter can be called multiple times, you’d want to avoid unnecessary markNeedsLayout calls.
To achieve that, always check that the new value is different from the current one.
class SomeRenderBox extends RenderBox {
double _dividerWidth;
double get dividerWidth => _dividerWidth;
set dividerWidth(double value) {
if (_dividerWidth == value) return;
_dividerWidth = value;
markNeedsLayout();
}
}to check for errors like this one, use the check-for-equals-in-render-object-setters rule.
When it comes to implementing updateRenderObject, make sure that you update all mutable fields that need to be updated. For example,
class _Decorator extends RenderObjectWidget {
const _Decorator({
required this.textDirection,
required this.isFocused,
required this.expands,
});
final TextDirection textDirection;
final bool isFocused;
final bool expands;
@override
_RenderDecoration createRenderObject(BuildContext context) {
return _RenderDecoration(
textDirection: textDirection,
isFocused: isFocused,
expands: expands,
);
}
@override
void updateRenderObject(
BuildContext context,
_RenderDecoration renderObject,
) {
renderObject
..expands = expands
..textDirection = textDirection;
}
}this RenderObjectWidget misses a field isFocused in the updateRenderObject implementation which leads to a bug when the field is not updated.
Instead, the updateRenderObject should look like:
@override
void updateRenderObject(
BuildContext context,
_RenderDecoration renderObject,
) {
renderObject
..expands = expands
..textDirection = textDirection
..isFocused = isFocused;
}To never forget to update all fields in updateRenderObject enable the consistent-update-render-object rule.
Ensuring proper types
Dart is a statically typed language and types are a useful tool to ensure your code works as you’d designed it to work (at least they should help), but sometimes you end up in a situation when the Dart type system goes against you.
Collection methods
Unfortunately, several collection methods (for example, contains, remove, difference and so on) are typed the way that the parameter is Object? (basically a dynamic). It create a space for some nasty mistakes when you occasionally pass a wrong type and there are no way to ensure that the type is correct, except manually.
For example,
final map = Map<int, String>();
map.containsKey("str");here we have a map that is checked for a “str” key in it. The problem is the map can only have int keys and the only way to notice that is to be very attentive.
To avoid mistakes like that and focus on more important things, use avoid-collection-methods-with-unrelated-types.
Avoiding dynamic type
Dynamic type creates too much room for mistakes and it’s advised to avoid it as long as possible (there are several valid use-cases for it, for example, when working with JSON payloads).
To ensure that you app does not rely on dynamic types, enable avoid-dynamic.
Unnecessary nullable return types
Introduction of null safety to Dart was a huge step forward for the language and a great build-in way to ensure what parts of your program can’t actually handle null. The problem is that if you mark all the code as nullable, you will quickly find yourself in a situation where you check for nullability and try to create a fallback branch of code that should be executed on a null value, but in reality the value you are working with is not expected to be null at all. That’s called an unnecessary nullable type.
The underlying problem is much worse: you’re not only writing the code that won’t ever be executed, you are also hiding a potential error (if a value actually appears to be null, you won’t see any error), writing tests for this code and leaving this code for other people to work with. That’s why it’s important to remove unnecessary nullability.
A simple example:
String? function() {
return 'srt';
}here the function can never ever return null.
Use avoid-unnecessary-nullable-return-type to avoid such problems.
Unnecessary operations with types
With the prepense of is and as expressions comes a problem when you try to compare the type with itself.
For example,
class Example {
final myList = <int>[1, 2, 3];
void main() {
final result = myList is List<int>;
}
}here myList is always a List<int> and no is check is needed. That’s why this operation is unnecessary. To avoid that, take a look at avoid-unnecessary-type-assertions and avoid-unnecessary-type-casts.
Unrelated operation with types
Another problem that comes with is and as is that you can try to use them with types that are not anyhow related, resulting in the operation to ever return false (or throw an error).
For example,
class Example {
final regularString = 'some string';
void main() {
final result = regularString is int;
}
}here regularString can never be of type int and the check can be removed.
To avoid problems like that, use avoid-unrelated-type-assertions and avoid-unrelated-type-casts.
Asynchronous code
While async code already has a higher learning curve and might be a bit hard to understand for the beginners, even experienced devs can fall into some non-obvious traps.
Try / catch and await
Let’s take a look at the following example:
Future<String> report() async {
try {
return anotherAsyncMethod();
} catch(e) {
reportError(e);
}
}We have an async function with a single try statement. If the anotherAsyncMethod function throws an error, we want this error to be handled by reportError. Looks simple and obvious, right?
But it actually contains a very tricky problem: since the anotherAsyncMethod is not awaited, even if the function throws an error, the catch won’t handle it and the error will be propagated outside of the report function.
This behavior is rarely a desired one and is hard to spot which might result in hours of debugging.
To avoid the need to check for the errors like this manually, you can use a rule called prefer-return-await.
So the correct version of the snipped above is:
Future<String> report() async {
try {
return await anotherAsyncMethod();
} catch(e) {
reportError(e);
}
}now even if the anotherAsyncMethod function throws an error, it will be correctly handled and passed to reportError.
Redundant async keyword
Next, let’s move on to another example:
Future<String> parseArgs(Iterable<String> args) async {
final argsWithDefaultCommand = _addDefaultCommand(args);
return _parse(argsWithDefaultCommand);
}
Future<String> _parse() {
...
}This code sample again contains an issue, can you spot it?
The problem is with the async keyword. It’s actually not needed here!
In Dart if a Future is not awaited inside a function, you can return it without marking the function async. Async function not only result in a more complex code, produced by the compiler, but also can prevent some optimizations like @pragma(‘vm:prefer-inline’) to take place. This pragma does not work for functions / methods that have try blocks, async, async* and sync*, but works just fine for functions / methods that have Future<> return type.
So, the correct version is:
Future<String> parseArgs(Iterable<String> args) {
final argsWithDefaultCommand = _addDefaultCommand(args);
return _parse(argsWithDefaultCommand);
}
Future<String> _parse() {
...
}And the DCM rule that can help with detecting async keyword usages that can be safely removed is called avoid-redundant-async.
Note, that there are some valid cases for async without await. For example, if you return a value that is not a Future. Adding an async keyword to a function like that allows the value to be automatically wrapped into the Future.
Async function for a parameter that expects a sync one
Last but not least: let’s say we have a button Widget with a VoidCallback field.
typedef VoidCallback = void Function();
class RawMaterialButton extends StatelessWidget {
const RawMaterialButton({
required this.onPressed,
});
final VoidCallback? onPressed;
void _pressedWrapper() {
onPressed();
// Execute some additional logic
}
Widget build(BuildContext context) {
return InkWell(
onTap: _pressedWrapper(),
);
}
}that is used in another Widget:
class AnotherWidget extends StatelessWidget {
Future<void> _someFuture() {
...
}
Widget build(BuildContext context) {
return RawMaterialButton(
onPressed: () async {
await _someFuture();
// Execute some additional logic
},
);
}
}Do you see the problem here?
Well, since the callback passed to onPressed returns Future<void> (which is totally acceptable by the Dart type system), but the void type is expected and the callback is not awaited in _pressedWrapper method, you might get into a race condition when _pressedWrapper completes before the callback passed to onPressed. This might lead to very tricky bugs like a callback being executed several time if the user taps too quickly on so on.
To highlight cases like that in your codebase, use the avoid-passing-async-when-sync-expected rule provided by DCM.
Proper usage of existing code
With a rich standard library and number of available widgets sometimes you still need to use some replacement of your own making. DCM rule called [banned-usage](https://dcm.dev/docs/teams/rules/common/banned-usage/) can help to ensure that the proper version is used. The capabilities of this rule allow you to ban both top level entities (like classes, enums, function) and specific methods of a class (like the `sort` method of `List`).
For example,
void main() {
final list = [1, 2, 3];
list.sort();
}the problem with the sort method is that it mutates the original list and this behavior might be the one you’d like to avoid.
To achieve that, you can configure this rule the following way:
dart_code_metrics:
...
rules:
...
- ban-name:
entries:
- type: List
entries:
- ident: sort
description: Use "sorted" insteadthis will highlight all calls of the List’s sort and suggest to use sorted (which comes from the collection package) and does not mutate the original list.
Another use-case for this rule is banning standard widgets.
Flutter’s flexibility in drawing literally anything you want comes with side effects like creating completely unique design and the standard widgets can rarely be used to bring this design to live.
You might find yourself in a situation when you have custom buttons that follow your design system, but the standard buttons are also available and you’d like to guide your team with what widgets should be used.
Here is an example configuration for banned-usage to achieve that:
dart_code_metrics:
...
rules:
...
- ban-name:
entries:
- ident: TextButton
description: "Please use SpecialButton in this package."This will not only highlight all usages of TextButton, but also provide a guidance what should be changed.
Readability
Considering that we as developers write the code not only for the compiler, but also for other developers (and usually you prioritise developers over the compiler). That’s why readability (especially on a long run) contributes that much to the maintainability. DCM can help with that too, so here are some cases to take into consideration.
Trailing commas
A trailing comma is a comma placed in the end of the nodes list (ex. parameter list).
Trailing commas take a special part in the Dart ecosystem due to how dart_style (or dart format) works. Based on the presence of the trailing comma a list will be formatted differently.
For example,
void thirdFunction(String someLongVarName, void Function() someLongCallbackName,
String arg3) {
...
}since arg3 does not have a comma after, the formatter tries to fit all the arguments into a single line.
But if we add a comma, here’s how the formatting changes:
void thirdFunction(
String someLongVarName,
void Function() someLongCallbackName,
String arg3,
) {
...
}Some may find the second one more readable. To enforce the presence of trailing commas, use the prefer-trailing-comma rule.
On the other hand, people tend to overuse the trailing commas even in the sort parameter lists. For example, if a list has only one parameter, formatted code with a trailing comma will look like:
void thirdFunction(
String someLongVarName,
) {
...
}whether without the comma, it fits in one line:
void thirdFunction(String someLongVarName) {
...
}Trailing commas like this one can be considered unnecessary and you may want to keep oneliners, where possible. To avoid unnecessary trailing commas enable the unnecessary-trailing-comma rule.
Callbacks in a widget tree
Since the build method of any widget (or its state) is the fastest way to answer the question of what parts the widget consists of, developers prefer to keep this method as simple as possible. There are two main contributors to the overall complexity: too many used widgets and large callbacks. Let’s focus on the second problem.
If your goal is to understand what parts a widget uses, reading through the large build methods where widgets and logic inside callback are mixed is not the type of activity you’d prefer. It’s totally okay to have small one-two line callbacks, they don’t increase the readability complexity that much. But when we talk about large callbacks like six plus lines of code — that should be a no go.
Let’s take a look at this example:
class MyWidget extends StatelessWidget {
@override
Widget build(BuildContext context) {
return TextButton(
style: ...,
onPressed: () {
// Some
// Huge
// Callback
},
child: ...
);
}
}instead, you should move callback like this from inline to a widget / state methods (preferably private).
class MyWidget extends StatelessWidget {
void _handlePressed() {
...
}
@override
Widget build(BuildContext context) {
return TextButton(
style: ...,
onPressed: _handlePressed,
child: ...
);
}
}The rule to help you maintain this practice is called prefer-extracting-callbacks.
Keeping one widget per file
Since the compiler does not care how many files you have and how well your code is formatted, this type of activity is made for other developers to better understand the code and be able to change it without breaking the whole system. Separating your widgets the way that each file has no more than one public widget is considered a good practice. Since the private widgets are a part (direct or transitive) of the public widget, separating widgets this way allows developers to avoid having too much files, but also keep all coherent widgets in one context for faster discoverability.
To enforce this policy for all widgets within an app, you can use the prefer-single-widget-per-file rule.
Reducing the code nesting
Code nesting refers to the concept of placing one structure within another. Deeply nested structures negatively affect readability and it’s a good practice to keep the number of levels no more than 4–5.
There are several approaches you can choose to reduce the nesting level. One is to return early from a code block based on a condition.
For example,
void someFunction() {
if (value == 2) {
...
if (anotherValue == 3) {
...
if (shouldContinue) {
// Some
// Calculations
}
}
}
}here each if statement introduces an additional nesting level which results in the desired calculations being deep in the nesting tree.
To avoid this, the code can be refactored to:
void someFunction() {
if (value != 2) {
return;
}
...
if (anotherValue != 3) {
return;
}
...
if (!shouldContinue) {
return;
}
// Some
// Calculations
}inverting the conditions and returning early allows us to keep the nesting level under control. DCM rule to highlight the code that can be refactored this way is called prefer-early-return.
Another practice to reduce the nesting level is to address else blocks in if statements. For example,
void withInstance() {
if (someCondition) {
// do some work
return;
} else {
// do some other work
}
...
}here the “then” branch of the if statement has a return which means that all code after it won’t be executed if the condition is evaluated to true.
This allows as to refactor the code removing the else completely and thus reducing the nesting level. Let’s take a look:
void withInstance() {
if (someCondition) {
// do some work
return;
}
// do some other work
...
}see how it does exactly the same as the code block before, but does not have an extra nesting level? A rule to enforce this code style is called avoid-redundant-else.
Memory leaks
Avoiding memory leaks is essential in a sense that debugging and reproducing such issues might take a significant amount of time. That’s why developers would prefer avoiding introducing them in the first place.
Unfortunately, not all memory leaks can be detected by static analysis tools, since the memory usage is a part of a program runtime and static analysis literally means “inspection without executing”. If you’re interested in more advanced tools to detect memory leaks, take a look at this one.
But there are still some common mistakes, that can be checked.
Disposing and removing listeners
If you app uses ChangeNotifiers, ValueListenable, ScrollControllers and other classes that implement the Listenable class, then you probably know, that you need to dispose or remove listeners when they are non needed.
To help you never forget, there are two rules always-remove-listener (focused on Listenable) and dispose-fields (for any widget’s State field of a type that has a dispose method).
For example,
class SomeDisposable implements Disposable {
@override
void dispose() {}
}
class _ShinyWidgetState extends State<ShinyWidget> {
final _someDisposable = SomeDisposable();
final _anotherDisposable = SomeDisposable();
void dispose() {
_someDisposable.dispose();
super.dispose();
}
}Even though the dispose method is correct and the super.dispose call is present, the _anotherDisposable field is not disposed which probably means that required cleanup work for this instance is not done which in its turn leads to a memory leak.
Always disposing fields like that is the first step you can take towards reducing the number of memory leaks your app has.
Bonus point: working with bytes
If your application manipulates with large amount of bytes, you will find this section useful.
Same as it’s advised to use a StringBuffer to avoid memory problems while working with large number of strings, it’s important to follow the same approach with bytes and use BytesBuilder instead of regular lists.
Just take a look at these benchmarks!
In short, spread operator and concatenation of large number of bytes will probably hang your program (or isolate), so code like this
void run() {
List<int> buffer = Uint8List(0);
for (var i = 0; i < 100; i++) {
buffer = buffer + chunks[i];
}
}should be rewritten to
void run() {
final buffer = BytesBuilder();
for (var i = 0; i < 100; i++) {
buffer.add(_chunks[i]);
}
}And a rule to enforce that is called prefer-bytes-builder.
Errors due to inattention
Sometimes due to deadlines or significant refactorings some of our code transforms into the code containing unused or unnecessary parts that does not make any sense. This code can also be detected by DCM, and its timely removal can noticeably reduce maintenance costs.
Unused parameters
For example, Dart does not provide a way to check if a function or a method has any parameters that are unused.
With DCM rule avoid-unused-parameters] it becomes pretty easy to spot them:
class SomeClass {
void method(String value, int counter) {
print(value);
}
}In this case the counter parameter is not used and can be removed without any side effects.
Complex conditions
Another type of problems comes when refactoring complex conditions.
For example, the following code:
final num = 1;
final anotherNum = 2;
void main() {
if (num == anotherNum && num == anotherNum) {
return;
}
}has a problem with both sides of the binary expression (&&) equal, meaning the code can be simplified to:
final num = 1;
final anotherNum = 2;
void main() {
if (num == anotherNum) {
return;
}
}and still give the same result.
There several rules DCM provides that can help you with complex conditions. avoid-self-compare and avoid-equal-expressions are two you need to pay attention to.
Missing invocations
Missing an invocation is another problem, that can appear due to refactoring.
For example,
class SomeClass {
final void Function() callback;
const SomeClass({
required this.callback,
});
}
void _handle() {
...
}
void main() {
SomeClass(
callback: () => _handle,
);
}since the return type of the callback function is void it opens a door for tricky bugs when the function inside a callback is not called, but should be. So, here is a correct version:
void main() {
SomeClass(
callback: () => _handle(),
);
}To avoid bugs like that, use avoid-missed-calls.
Copy with
Naming copyWith methods that return an updated entity of an immutable class is an adopted by the community practice which works well. The problem arises when you update a class that has this method and forget to update the method accordingly (especially, if the class is a part of your package public API). Instead, you can use avoid-incomplete-copy-with and never face this problem again.
For example,
class Person {
const Person({
required this.name,
required this.surname,
});
final String name;
final String surname;
Person copyWith({String? name}) {
return Person(
name: name ?? this.name,
surname: surname,
);
}
}here we have a class named Person with two fields. If you intend to create a copy of an instance of this class, you’d probably need an ability to change both of the fields. But here, the copyWith method takes only one parameter, thus forcing us to manually create a Person instance if we want to update surname. In this particular class it sounds like not a big deal, but if a class has like 20 fields and at least on of them is not possible to update via copyWith, this might be a problem resulting in a lot of boilerplate code.
Shadowing
Variable shadowing occurs when a variable declared within a certain scope (decision block, method, or inner class) has the same name as a variable declared in an outer scope.
The problem it brings (especially in large code blocks) is that you might think that you refer to one variable when in reality there is another variable in the scope. That’s why it’s generally advised to avoid shadowing and keep all names of the variables within a scope unique.
DCM rule to highlight all shadowing problems in your code is called avoid-shadowing.
That’s how it looks in code:
class SomeClass {
final String content;
...
void calculate(String content) {
...
}
}here, if you write code within the calculate method and refer to content you will always refer to the parameters of the method unless you explicitly use this.content.
Super calls order
If you ever had problems with initState and dispose super calls, DCM can help you with that as well!
The order of super calls in these methods is essential since they are a part of widgets lifecycle and incorrect (or even missing) calls can result in memory leaks and degraded app performance (which you’d probably like to avoid).
With the proper-super-calls rule you can stop double checking and keeping in mind what is the proper order of the super call — DCM will cover that for you!
For example,
class _MyHomePageState<T> extends State<MyHomePage> {
@override
void dispose() {
super.dispose();
someDisposeWork();
}
}this code example may look normal, but it contains the problem of incorrect super order. If there is any work in someDisposeWork that releases resources, it might never be completed!
Bonus point: Equatable
If you use the Equatable package, then you might be familiar with the requirement to list all the properties in the props:
class AnotherPerson extends Equatable {
const AnotherPerson(this.name, this.age);
final String name;
final int age;
@override
List<Object> get props => [name];
}the problem with this approach is that it’s toooo easy to forget to update the props getter when a new fields is added to the class.
If you ever had this problem, say no more. DCM has you covered here and provides list-all-equatable-fields. You won’t ever again forget to add a field to the props getter!
Conclusion
Some of the mistakes we discussed above are very tricky! You need to be aware of them and remember that they exist in order to spot them without hours of debugging. But there is no need for you to rely only on yourself. Automated checks can save you a ton of time and validate a lot of different issues for you, don’t refuse from their help!
