✨ Bad Code Example #2
Conversation
DCM report❌ warning issues: 32, style issues: 20, perf issues: 2 Full report: https://github.com/aditya-css/counter_dcm/runs/25450637058 |
aditya-css
force-pushed
the
feature/bad_example
branch
3 times, most recently
from
6a6755a to
7e8e8e5
Compare
aditya-css
force-pushed
the
feature/bad_example
branch
from
7e8e8e5 to
f3765cf
Compare
WalkthroughThe changes introduce multiple steps in the GitHub Actions workflow for running Developer Code Metrics (DCM) analyses, enhancing code quality checks. Additionally, a new Flutter widget Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| void doSomething(int a, int b, int c, int d, int e) { | ||
| if (e > 100) return; | ||
| if (a > 0) { | ||
| if (b > 0) { | ||
| if (c > 0) { | ||
| if (d > 0) { | ||
| print('All positive'); | ||
| } else { | ||
| print('d is not positive'); | ||
| } | ||
| } else { | ||
| print('c is not positive'); | ||
| } | ||
| } else { | ||
| print('b is not positive'); | ||
| } | ||
| } else { | ||
| print('a is not positive'); | ||
| } | ||
| } |
There was a problem hiding this comment.
The doSomething method has a deeply nested structure due to multiple if statements, which makes it hard to read and maintain. Consider refactoring this method to reduce nesting, possibly by using early returns or extracting methods. This will improve the readability and maintainability of the code.
| void complexMethod() { | ||
| doSomething(1, 2, 3, 4, 5); | ||
| method1(); | ||
| method2(); | ||
| method3(); | ||
| method4(); | ||
| method5(); | ||
| method6(); | ||
| method7(); | ||
| method8(); | ||
| method9(); | ||
| method10(); | ||
| doSomething(1, 2, 3, 4, 5); | ||
| doSomething(1, 2, 3, 4, 5); | ||
| } |
There was a problem hiding this comment.
The complexMethod method sequentially calls multiple methods, including doSomething three times with the same arguments, which could indicate unnecessary repetition or a lack of abstraction. If the intention is to perform a series of operations that are logically related, consider encapsulating these operations within a more appropriately named method or class that clearly defines their purpose. This will enhance the modularity and clarity of the code.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
lib/bad_example.dart (1)
10-10: Consider making these methods private if they are not intended to be accessed outside of this class.Also applies to: 31-31, 35-35, 39-39, 43-43, 47-47, 51-51, 55-55, 59-59, 63-63, 67-67, 71-71, 145-145
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/code-quality.yaml (1 hunks)
- lib/bad_example.dart (1 hunks)
Additional Context Used
GitHub Check Runs (1)
DCM report failure (30)
lib/bad_example.dart: [notice] 9-9: lib/bad_example.dart#L9
Prefer formatting comments like sentences.
lib/bad_example.dart: [warning] 3-3: lib/bad_example.dart#L3
The first class name does not match the file name. Try renaming it.
lib/bad_example.dart: [notice] 3-3: lib/bad_example.dart#L3
Prefer declaring a const constructor.
lib/bad_example.dart: [notice] 10-10: lib/bad_example.dart#L10
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 31-31: lib/bad_example.dart#L31
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 35-35: lib/bad_example.dart#L35
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 39-39: lib/bad_example.dart#L39
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 43-43: lib/bad_example.dart#L43
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 47-47: lib/bad_example.dart#L47
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 51-51: lib/bad_example.dart#L51
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 55-55: lib/bad_example.dart#L55
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 59-59: lib/bad_example.dart#L59
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 63-63: lib/bad_example.dart#L63
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 67-67: lib/bad_example.dart#L67
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 71-71: lib/bad_example.dart#L71
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [notice] 145-145: lib/bad_example.dart#L145
Prefer declaring widget or state class members as private.
lib/bad_example.dart: [warning] 11-11: lib/bad_example.dart#L11
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 72-72: lib/bad_example.dart#L72
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 72-72: lib/bad_example.dart#L72
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 72-72: lib/bad_example.dart#L72
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 72-72: lib/bad_example.dart#L72
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 83-83: lib/bad_example.dart#L83
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 83-83: lib/bad_example.dart#L83
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 83-83: lib/bad_example.dart#L83
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 83-83: lib/bad_example.dart#L83
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 84-84: lib/bad_example.dart#L84
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 84-84: lib/bad_example.dart#L84
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 84-84: lib/bad_example.dart#L84
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [warning] 84-84: lib/bad_example.dart#L84
Avoid magic numbers. Try extracting them to named constants or variables.
lib/bad_example.dart: [notice] 91-91: lib/bad_example.dart#L91
Avoid unnecessary trailing comma. Try removing it.
Additional comments not posted (7)
.github/workflows/code-quality.yaml (7)
31-38: The configuration for "Run DCM Analyze" is correctly set up with appropriate secrets and action version.
39-41: The "Run DCM Code Duplication" step is correctly configured to check for code duplication in thelibdirectory.
42-44: The "Run DCM Check Parameters" step is properly set up to analyze parameter usage in thelibdirectory.
45-48: The steps for checking unused code and files are well-configured to target thelibdirectory.
49-51: The "Run DCM Check Dependencies" step is correctly configured to analyze dependencies in thelibdirectory.
52-54: The "Run DCM Metrics Calculation" step is properly set up to calculate metrics and output them in HTML format.
55-56: The "Run DCM Widget Analyses" step is correctly configured to analyze widgets in thelibdirectory.
| @@ -0,0 +1,152 @@ | |||
| import 'package:flutter/material.dart'; | |||
|
|
|||
| class BadFlutterExample extends StatefulWidget { | |||
There was a problem hiding this comment.
Consider renaming the class to match the file name or vice versa to improve clarity and maintainability.
Consider adding a const constructor to improve performance by allowing widgets to be compile-time constants when possible.
| class _BadFlutterExampleState extends State<BadFlutterExample> { | ||
| // Many parameters | ||
| void doSomething(int a, int b, int c, int d, int e) { | ||
| if (e > 100) return; |
There was a problem hiding this comment.
Consider extracting magic numbers into named constants for better readability and maintainability.
Also applies to: 72-72, 83-83, 84-84
![simformreviewbot[bot]](https://avatars.githubusercontent.com/u/98031138?s=60&v=4){kind=small}
| import 'package:flutter/material.dart'; | ||
|
|
||
| class BadFlutterExample extends StatefulWidget { | ||
| @override | ||
| State<BadFlutterExample> createState() => _BadFlutterExampleState(); | ||
| } | ||
|
|
||
| class _BadFlutterExampleState extends State<BadFlutterExample> { | ||
| // Many parameters | ||
| void doSomething(int a, int b, int c, int d, int e) { | ||
| if (e > 100) return; | ||
| if (a > 0) { | ||
| if (b > 0) { | ||
| if (c > 0) { | ||
| if (d > 0) { | ||
| print('All positive'); | ||
| } else { | ||
| print('d is not positive'); | ||
| } | ||
| } else { | ||
| print('c is not positive'); | ||
| } | ||
| } else { | ||
| print('b is not positive'); | ||
| } | ||
| } else { | ||
| print('a is not positive'); | ||
| } | ||
| } | ||
|
|
||
| void method1() { | ||
| print('method1'); | ||
| } | ||
|
|
||
| void method2() { | ||
| print('method2'); | ||
| } | ||
|
|
||
| void method3() { | ||
| print('method3'); | ||
| } | ||
|
|
||
| void method4() { | ||
| print('method4'); | ||
| } | ||
|
|
||
| void method5() { | ||
| print('method5'); | ||
| } | ||
|
|
||
| void method6() { | ||
| print('method6'); | ||
| } | ||
|
|
||
| void method7() { | ||
| print('method7'); | ||
| } | ||
|
|
||
| void method8() { | ||
| print('method8'); | ||
| } | ||
|
|
||
| void method9() { | ||
| print('method9'); | ||
| } | ||
|
|
||
| void method10() { | ||
| print('method10'); | ||
| } | ||
|
|
||
| void complexMethod() { | ||
| doSomething(1, 2, 3, 4, 5); | ||
| method1(); | ||
| method2(); | ||
| method3(); | ||
| method4(); | ||
| method5(); | ||
| method6(); | ||
| method7(); | ||
| method8(); | ||
| method9(); | ||
| method10(); | ||
| doSomething(1, 2, 3, 4, 5); | ||
| doSomething(1, 2, 3, 4, 5); | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return Scaffold( | ||
| appBar: AppBar( | ||
| title: Text('Bad Flutter Example'), | ||
| ), | ||
| body: SingleChildScrollView( | ||
| child: Column( | ||
| children: [ | ||
| for (int i = 0; i < 10; i++) | ||
| Container( | ||
| margin: EdgeInsets.all(10), | ||
| child: Column( | ||
| children: [ | ||
| Row( | ||
| children: [ | ||
| Text('Item $i'), | ||
| Icon(Icons.star), | ||
| ElevatedButton( | ||
| onPressed: () {}, | ||
| child: Text('Button $i'), | ||
| ), | ||
| ], | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| Text('Counter: 0'), | ||
| ElevatedButton( | ||
| onPressed: () { | ||
| complexMethod(); | ||
| }, | ||
| child: Text('Do Something'), | ||
| ), | ||
| for (int i = 0; i < 10; i++) | ||
| Container( | ||
| margin: EdgeInsets.all(10), | ||
| child: Column( | ||
| children: [ | ||
| Row( | ||
| children: [ | ||
| Text('Item $i'), | ||
| Icon(Icons.star), | ||
| ElevatedButton( | ||
| onPressed: () {}, | ||
| child: Text('Button $i'), | ||
| ), | ||
| ], | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| void testMethod() { | ||
| method1(); | ||
| method2(); | ||
| method3(); | ||
| method4(); | ||
| method5(); | ||
| } | ||
| } |
There was a problem hiding this comment.
- Description: Nested if statements make the code hard to read and understand
- Impact: Reduced maintainability and potential for bugs
- Recommendation: Use a single if statement with logical operators: void doSomething(int a, int b, int c, int d, int e) {
if (e > 100 || a <= 0 || b <= 0 || c <= 0 || d <= 0) return;
print('All positive');
}
| import 'package:flutter/material.dart'; | ||
|
|
||
| class BadFlutterExample extends StatefulWidget { | ||
| @override | ||
| State<BadFlutterExample> createState() => _BadFlutterExampleState(); | ||
| } | ||
|
|
||
| class _BadFlutterExampleState extends State<BadFlutterExample> { | ||
| // Many parameters | ||
| void doSomething(int a, int b, int c, int d, int e) { | ||
| if (e > 100) return; | ||
| if (a > 0) { | ||
| if (b > 0) { | ||
| if (c > 0) { | ||
| if (d > 0) { | ||
| print('All positive'); | ||
| } else { | ||
| print('d is not positive'); | ||
| } | ||
| } else { | ||
| print('c is not positive'); | ||
| } | ||
| } else { | ||
| print('b is not positive'); | ||
| } | ||
| } else { | ||
| print('a is not positive'); | ||
| } | ||
| } | ||
|
|
||
| void method1() { | ||
| print('method1'); | ||
| } | ||
|
|
||
| void method2() { | ||
| print('method2'); | ||
| } | ||
|
|
||
| void method3() { | ||
| print('method3'); | ||
| } | ||
|
|
||
| void method4() { | ||
| print('method4'); | ||
| } | ||
|
|
||
| void method5() { | ||
| print('method5'); | ||
| } | ||
|
|
||
| void method6() { | ||
| print('method6'); | ||
| } | ||
|
|
||
| void method7() { | ||
| print('method7'); | ||
| } | ||
|
|
||
| void method8() { | ||
| print('method8'); | ||
| } | ||
|
|
||
| void method9() { | ||
| print('method9'); | ||
| } | ||
|
|
||
| void method10() { | ||
| print('method10'); | ||
| } | ||
|
|
||
| void complexMethod() { | ||
| doSomething(1, 2, 3, 4, 5); | ||
| method1(); | ||
| method2(); | ||
| method3(); | ||
| method4(); | ||
| method5(); | ||
| method6(); | ||
| method7(); | ||
| method8(); | ||
| method9(); | ||
| method10(); | ||
| doSomething(1, 2, 3, 4, 5); | ||
| doSomething(1, 2, 3, 4, 5); | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return Scaffold( | ||
| appBar: AppBar( | ||
| title: Text('Bad Flutter Example'), | ||
| ), | ||
| body: SingleChildScrollView( | ||
| child: Column( | ||
| children: [ | ||
| for (int i = 0; i < 10; i++) | ||
| Container( | ||
| margin: EdgeInsets.all(10), | ||
| child: Column( | ||
| children: [ | ||
| Row( | ||
| children: [ | ||
| Text('Item $i'), | ||
| Icon(Icons.star), | ||
| ElevatedButton( | ||
| onPressed: () {}, | ||
| child: Text('Button $i'), | ||
| ), | ||
| ], | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| Text('Counter: 0'), | ||
| ElevatedButton( | ||
| onPressed: () { | ||
| complexMethod(); | ||
| }, | ||
| child: Text('Do Something'), | ||
| ), | ||
| for (int i = 0; i < 10; i++) | ||
| Container( | ||
| margin: EdgeInsets.all(10), | ||
| child: Column( | ||
| children: [ | ||
| Row( | ||
| children: [ | ||
| Text('Item $i'), | ||
| Icon(Icons.star), | ||
| ElevatedButton( | ||
| onPressed: () {}, | ||
| child: Text('Button $i'), | ||
| ), | ||
| ], | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| void testMethod() { | ||
| method1(); | ||
| method2(); | ||
| method3(); | ||
| method4(); | ||
| method5(); | ||
| } | ||
| } |
There was a problem hiding this comment.
- Description: Redundant code duplication
- Impact: Wastes space and increases maintenance effort
- Recommendation: Refactor into a helper method: void doSomething(int a, int b, int c, int d, int e) {
if (isPositive(e) && isPositive(a) && isPositive(b) && isPositive(c) && isPositive(d)) {
print('All positive');
}
}
bool isPositive(int value) => value > 0;
|
| import 'package:flutter/material.dart'; | ||
|
|
||
| class BadFlutterExample extends StatefulWidget { | ||
| @override | ||
| State<BadFlutterExample> createState() => _BadFlutterExampleState(); | ||
| } | ||
|
|
||
| class _BadFlutterExampleState extends State<BadFlutterExample> { | ||
| // Many parameters | ||
| void doSomething(int a, int b, int c, int d, int e) { | ||
| if (e > 100) return; | ||
| if (a > 0) { | ||
| if (b > 0) { | ||
| if (c > 0) { | ||
| if (d > 0) { | ||
| print('All positive'); | ||
| } else { | ||
| print('d is not positive'); | ||
| } | ||
| } else { | ||
| print('c is not positive'); | ||
| } | ||
| } else { | ||
| print('b is not positive'); | ||
| } | ||
| } else { | ||
| print('a is not positive'); | ||
| } | ||
| } | ||
|
|
||
| void method1() { | ||
| print('method1'); | ||
| } | ||
|
|
||
| void method2() { | ||
| print('method2'); | ||
| } | ||
|
|
||
| void method3() { | ||
| print('method3'); | ||
| } | ||
|
|
||
| void method4() { | ||
| print('method4'); | ||
| } | ||
|
|
||
| void method5() { | ||
| print('method5'); | ||
| } | ||
|
|
||
| void method6() { | ||
| print('method6'); | ||
| } | ||
|
|
||
| void method7() { | ||
| print('method7'); | ||
| } | ||
|
|
||
| void method8() { | ||
| print('method8'); | ||
| } | ||
|
|
||
| void method9() { | ||
| print('method9'); | ||
| } | ||
|
|
||
| void method10() { | ||
| print('method10'); | ||
| } | ||
|
|
||
| void complexMethod() { | ||
| doSomething(1, 2, 3, 4, 5); | ||
| method1(); | ||
| method2(); | ||
| method3(); | ||
| method4(); | ||
| method5(); | ||
| method6(); | ||
| method7(); | ||
| method8(); | ||
| method9(); | ||
| method10(); | ||
| doSomething(1, 2, 3, 4, 5); | ||
| doSomething(1, 2, 3, 4, 5); | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return Scaffold( | ||
| appBar: AppBar( | ||
| title: Text('Bad Flutter Example'), | ||
| ), | ||
| body: SingleChildScrollView( | ||
| child: Column( | ||
| children: [ | ||
| for (int i = 0; i < 10; i++) | ||
| Container( | ||
| margin: EdgeInsets.all(10), | ||
| child: Column( | ||
| children: [ | ||
| Row( | ||
| children: [ | ||
| Text('Item $i'), | ||
| Icon(Icons.star), | ||
| ElevatedButton( | ||
| onPressed: () {}, | ||
| child: Text('Button $i'), | ||
| ), | ||
| ], | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| Text('Counter: 0'), | ||
| ElevatedButton( | ||
| onPressed: () { | ||
| complexMethod(); | ||
| }, | ||
| child: Text('Do Something'), | ||
| ), | ||
| for (int i = 0; i < 10; i++) | ||
| Container( | ||
| margin: EdgeInsets.all(10), | ||
| child: Column( | ||
| children: [ | ||
| Row( | ||
| children: [ | ||
| Text('Item $i'), | ||
| Icon(Icons.star), | ||
| ElevatedButton( | ||
| onPressed: () {}, | ||
| child: Text('Button $i'), | ||
| ), | ||
| ], | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| void testMethod() { | ||
| method1(); | ||
| method2(); | ||
| method3(); | ||
| method4(); | ||
| method5(); | ||
| } | ||
| } |
There was a problem hiding this comment.
- Description: No null check on user.displayName, potential runtime error.
- Impact: If the 'user' object is null, accessing its 'displayName' property will cause a runtime error. This violates Dart's null safety principles.
- Recommendation: Add a null check before accessing user.displayName. For example: final String userName = user?.displayName ?? 'Guest';
- Severity: High
Summary by CodeRabbit
New Features
BadFlutterExampledemonstrating a complex UI structure.Chores