#Lecture 127 — Dart Code Review

The formatter, the analyzer and your tests catch a lot, but they cannot tell you that a rule is wrong or that a secret is printed. A code review puts a second pair of eyes on the code before it ships.

Contents

Prerequisites · Connection · Objectives · Lesson · Code Examples · Common Mistakes · Best Practices · Mini Challenge · Practice Questions · Answers · Takeaways

Every error message in this lecture is real. Each one was produced by running the code shown on Dart 3.12.2, so you will recognize it when you meet it yourself.

#Prerequisites

Students should already understand, from Lectures 015–126:

• the null assertion operator ! and null checks (Lectures 015–016)
• int.tryParse (Lecture 016) and try/on/catch (Lecture 084)
• dart format, dart analyze and lints (Lectures 114 and 117)
• writing tests with package:test (Lectures 118–122)
• clean code, documentation and performance habits (Lectures 123–126)

#Connection With Previous Lectures

Here is a line that passes dart format, passes dart analyze, and runs:

if (average > 80) return 'A'; // the rule says "80 and above"

No tool knows the school's rule, so a student with exactly 80 quietly gets a B. The tools from Lectures 114–122 check that code is well formed; only a person can check that it is right. This lecture is about that person, and how to be a good one.

#Learning Objectives

By the end of this lecture you will be able to:

• Explain what a code review is and why teams use it
• Review your own code first
• Apply a Dart review checklist: correctness, null safety, errors, naming, tests, tools, performance, security
• Spot an unnecessary !, a swallowed exception and a logged secret
• Write review comments that are kind, specific and labelled
• Receive feedback without taking it personally
• Carry out a full review: find problems, comment, and fix

#1. What Is Code Review?

A code review is when someone reads your change before it joins the main code and either approves it or asks for changes.

Analogy — it is like asking a friend to read your essay before you hand it in. You know what you meant to write; they see what you actually wrote.

On a team this usually happens on a pull request, with comments next to specific lines.

#2. Why Review Code?

• Bugs are cheaper now. Found in review, a bug costs one comment; found by a user, it costs a bug report, a fix and an apology.
• Knowledge spreads. At least two people understand every part of the code.
• The code stays consistent in names, structure and error handling.

The goal is better code, not proving who is smarter.

#3. Review Yourself First

Before anyone else spends time on your change, run the tools and read your own diff:

$ dart format .
$ dart analyze
$ dart test

Then read every changed line as if a stranger wrote it, removing debug prints and commented-out code. Your reviewer's time is for problems tools cannot see.

#4. The Review Checklist

The next sections show the four problems reviewers meet most often.

#5. Edge Cases: The Empty List

void main() {
  final marks = <int>[];
  var total = 0;
  for (final mark in marks) {
    total += mark;
  }
  print(total / marks.length);
}

NaN

0 / 0 is not an error in Dart; it is NaN ("not a number"), which silently flows onward. A reviewer asks: what if there are no marks?

#6. Null Safety: The Unnecessary !

Every ! is a promise that the value is never null. A map lookup cannot keep that promise:

void main() {
  final prices = {'Pen': 50, 'Bag': 1500};
  final price = prices['Book']!;
  print(price);
}

Unhandled exception:
Null check operator used on a null value

Handle the null instead:

void main() {
  final prices = {'Pen': 50, 'Bag': 1500};
  final price = prices['Book'];
  if (price == null) {
    print('Book is not on sale');
  } else {
    print('Book costs $price');
  }
}

Book is not on sale

#7. Error Handling: The Swallowed Exception

int parseMark(String text) {
  try {
    return int.parse(text);
  } catch (e) {}
  return 0;
}

void main() {
  print(parseMark('8O'));
}

0

A teacher typed the letter O instead of zero, and the program turned it into a mark of 0 without a word. The analyzer flags the empty block as empty_catches, but only as an info. int.tryParse returns null for bad input, so the caller is forced to decide:

void main() {
  final mark = int.tryParse('8O');
  if (mark == null) {
    print('"8O" is not a number');
  } else {
    print('Mark: $mark');
  }
}

"8O" is not a number

#8. Security: Never Log Secrets

print('Logging in $user with password $password'); // ✗
print('Logging in $user'); // ✓

Printed text ends up in log files and screenshots. Never print passwords, API keys, PINs or full card numbers, and keep keys out of source code.

#9. Giving Kind, Specific Feedback

Comment on the code, not the person; say why; suggest a fix.

Label how much each comment matters: blocking (must change), suggestion (worth considering), nit (tiny, optional), question (help me understand). And say what is good — "nice test for the empty list" is a review comment too.

#10. Receiving Feedback

• A comment about your code is not a comment about you.
• Assume good intent; ask if a comment is unclear.
• Fix it, or politely explain why not.
• Reply to every comment, and thank the reviewer.

#11. A Real-World Example

Ali asks Sara to review his class report. It is formatted and it runs:

// Grades: A for an average of 80 and above, B for 50 and above, else C.
const apiKey = 'sk-live-12345';

class student {
  String name;
  List<int> marks;
  student(this.name, this.marks);
}

double avg(List<int> m) {
  var total = 0;
  for (var i = 0; i <= m.length - 1; i++) {
    total += m[i];
  }
  return total / m.length;
}

String grade(double a) {
  if (a > 80) return 'A';
  if (a > 50) return 'B';
  return 'C';
}

int parseMark(String text) {
  try {
    return int.parse(text);
  } catch (e) {}
  return 0;
}

void main() {
  print('Connecting to the marks server with key $apiKey');

  var Students = <String, student>{
    'Ali': student('Ali', [85, 90, 78]),
    'Sara': student('Sara', [80, 80]),
    'Zayan': student('Zayan', []),
  };

  // A mark typed in by a teacher.
  Students['Sara']!.marks.add(parseMark('8O'));

  for (var name in Students.keys) {
    var s = Students[name]!;
    print('$name: ${avg(s.marks)} ${grade(avg(s.marks))}');
  }
}

Connecting to the marks server with key sk-live-12345
Ali: 84.33333333333333 A
Sara: 53.333333333333336 B
Zayan: NaN C

$ dart analyze
Analyzing class_report...

   info - bin/report.dart:4:7 - The type name 'student' isn't an UpperCamelCase identifier. Try changing the name to follow the UpperCamelCase style. - camel_case_types
   info - bin/report.dart:27:15 - Empty catch block. Try adding statements to the block, adding a comment to the block, or removing the 'catch' clause. - empty_catches
   info - bin/report.dart:34:7 - The variable name 'Students' isn't a lowerCamelCase identifier. Try changing the name to follow the lowerCamelCase style. - non_constant_identifier_names

3 issues found.

The analyzer found three style problems. Sara's review finds the real ones:

Ali replies to each comment ("Good catch on the 80 boundary, thanks!") and pushes this:

// Grades: A for an average of 80 and above, B for 50 and above, else C.

class Student {
  final String name;
  final List<int> marks;

  Student(this.name, this.marks);

  /// The average mark, or `null` when there are no marks yet.
  double? get average {
    if (marks.isEmpty) return null;
    var total = 0;
    for (final mark in marks) {
      total += mark;
    }
    return total / marks.length;
  }
}

String gradeFor(double average) {
  if (average >= 80) return 'A';
  if (average >= 50) return 'B';
  return 'C';
}

/// Adds [text] to [student]'s marks, or reports it if it is not a number.
void addMark(Student student, String text) {
  final mark = int.tryParse(text);
  if (mark == null) {
    print('Skipped invalid mark "$text" for ${student.name}');
    return;
  }
  student.marks.add(mark);
}

void main() {
  print('Connecting to the marks server...');

  final sara = Student('Sara', [80, 80]);
  final students = [
    Student('Ali', [85, 90, 78]),
    sara,
    Student('Zayan', []),
  ];

  // A mark typed in by a teacher.
  addMark(sara, '8O');

  for (final student in students) {
    final average = student.average;
    if (average == null) {
      print('${student.name}: no marks yet');
    } else {
      final shown = average.toStringAsFixed(1);
      print('${student.name}: $shown (${gradeFor(average)})');
    }
  }
}

Connecting to the marks server...
Skipped invalid mark "8O" for Sara
Ali: 84.3 (A)
Sara: 80.0 (A)
Zayan: no marks yet

$ dart analyze
Analyzing class_report...
No issues found!

Sara gets the A she earned, Zayan is not graded on nothing, and the key is gone. The tests she asked for are Question 10. Notice that the analyzer pointed at only one of the four real bugs, the empty catch, and only as an info. The logged key, the NaN and the boundary were found by a person.

#Code Examples

Each example is the "after review" version, with the review comment it answers.

#Example 1 — A Default Instead of !

void main() {
  final stock = {'Pen': 12, 'Bag': 3};
  // Review: `stock['Book']!` would crash. A default is safer.
  final books = stock['Book'] ?? 0;
  print('Books in stock: $books');
}

Books in stock: 0

#Example 2 — for-in Instead of an Index Loop

void main() {
  final names = ['Ali', 'Sara', 'Zayan'];
  // Review: index loop `i <= names.length - 1` replaced with for-in.
  for (final name in names) {
    print('Hello, $name');
  }
}

Hello, Ali
Hello, Sara
Hello, Zayan

#Example 3 — Guarding the Empty List

double? average(List<int> marks) {
  if (marks.isEmpty) return null; // Review: no more NaN.
  var total = 0;
  for (final mark in marks) {
    total += mark;
  }
  return total / marks.length;
}

void main() {
  print(average([70, 80]));
  print(average([]));
}

75.0
null

#Example 4 — The Boundary

// Review: "50 and above passes" means >=, not >.
bool passed(int mark) => mark >= 50;

void main() {
  print(passed(49));
  print(passed(50));
  print(passed(51));
}

false
true
true

#Example 5 — tryParse Instead of an Empty catch

int? parseAge(String text) {
  final age = int.tryParse(text); // Review: no empty catch.
  if (age == null || age < 0) return null;
  return age;
}

void main() {
  print(parseAge('20'));
  print(parseAge('twenty'));
  print(parseAge('-3'));
}

20
null
null

#Example 6 — Catching a Specific Exception

void main() {
  try {
    int.parse('12a');
  } on FormatException catch (e) {
    // Review: catch the specific type and say what happened.
    print('Bad number: ${e.source}');
  }
}

Bad number: 12a

#Example 7 — Student Marks

String gradeFor(int mark) {
  if (mark < 0 || mark > 100) {
    throw ArgumentError.value(mark, 'mark', 'must be 0 to 100');
  }
  if (mark >= 80) return 'A';
  if (mark >= 50) return 'B';
  return 'C';
}

void main() {
  for (final mark in [80, 79, 50, 49]) {
    print('$mark -> ${gradeFor(mark)}');
  }
}

80 -> A
79 -> B
50 -> B
49 -> C

#Example 8 — Shopping Total

void main() {
  final prices = {'Notebook': 500, 'Pen': 50};
  final cart = ['Notebook', 'Pen', 'Ruler'];
  var total = 0;
  for (final item in cart) {
    final price = prices[item]; // Review: no `!` on a lookup.
    if (price == null) {
      print('No price for $item, skipped');
      continue;
    }
    total += price;
  }
  print('Total: $total');
}

No price for Ruler, skipped
Total: 550

#Example 9 — Bank Balance

class Account {
  final String owner;
  final String pin;
  int balance;

  Account(this.owner, this.pin, this.balance);

  // Review: never print the PIN, even while debugging.
  @override
  String toString() => 'Account($owner, balance: $balance)';
}

void main() {
  final account = Account('Ali', '4321', 1000);
  print(account);
}

Account(Ali, balance: 1000)

#Example 10 — Temperature

void main() {
  final celsius = [18.0, 22.5, 25.0];
  // Review: the factor was recalculated inside the loop; do it once.
  const factor = 9 / 5;
  for (final c in celsius) {
    print('$c°C = ${c * factor + 32}°F');
  }
}

18.0°C = 64.4°F
22.5°C = 72.5°F
25.0°C = 77.0°F

#Common Beginner Mistakes

1. Trusting a clean analyzer — No issues found! means well formed, not correct. Ali's report had four real bugs, and the analyzer hinted at just one.

2. Using ! to silence the analyzer

// final price = prices['Book']!;

Unhandled exception:
Null check operator used on a null value

3. Swallowing exceptions — an empty catch (e) {} turns a crash you would notice into wrong data you will not.

4. Vague or harsh comments — "this is bad" gives nothing to act on.

5. Huge pull requests — nobody reviews 2,000 lines carefully. Keep changes small.

6. Arguing about style — the formatter and lints already settle it.

#Best Practices

• Self-review with dart format, dart analyze and dart test first.
• Keep each change small.
• Review with the checklist: correctness, edges, null safety, errors, naming, tests, tools, performance, security.
• Explain why and suggest a fix; label comments blocking, suggestion, nit or question.
• Reply to every comment, and thank your reviewer.

#Mini Challenge

Review this program: list its problems, then write the fixed version.

void main() {
  var Prices = {'Pen': 50};
  var p = Prices['Bag']!;
  print('Total: ${p * 2}');
}

Try it yourself before opening the solution.

void main() {
  final prices = {'Pen': 50};
  final bagPrice = prices['Bag'];
  if (bagPrice == null) {
    print('Bag has no price yet');
  } else {
    print('Total: ${bagPrice * 2}');
  }
}

Bag has no price yet

#Practice Questions

Beginner

1. Name the three commands you should run before asking for a review.
2. Rewrite "This is wrong." as a kind, specific comment on if (mark > 50) when the rule is "50 and above passes".
3. Give two kinds of problems dart analyze cannot find.

Application

4. final ages = {'Ali': 20}; — print Zayan's age, or unknown if he is missing, without !.
5. Rewrite parseMark from section 7 to return int? using int.tryParse. Test it with '75' and '7S'.
6. A program prints print('Card: $cardNumber'); for '4111222233334444'. Fix it to print only the last four digits.
7. Label each comment blocking, suggestion or nit: (a) "This prints the password." (b) "A trailing space in this message." (c) "A Set might be faster here, but the list has five items."

Challenge

8. Review and fix:

   dartCopy

   // double avgTemp(List<double> t) {
   //   var s = 0.0;
   //   for (var i = 0; i < t.length; i++) s += t[i];
   //   return s / t.length;
   // }

9. A reviewer writes: "This loop recalculates the total every time." You disagree, because it runs once on three items. Write a polite reply.

10. Sara asked for tests. Put gradeFor from section 11 in lib/grades.dart and test both sides of 80 and 50.

#Practice Answers

#Question 1 — Solution

dart format ., dart analyze and dart test.

#Question 2 — Solution

"The rule says 50 and above passes, but > 50 fails a mark of exactly 50. Could this be >= 50?"

#Question 3 — Solution

Wrong logic, such as a boundary or a missed empty list, and security problems, such as a printed password.

#Question 4 — Solution

void main() {
  final ages = {'Ali': 20};
  final age = ages['Zayan'];
  print(age == null ? 'unknown' : '$age');
}

unknown

#Question 5 — Solution

int? parseMark(String text) => int.tryParse(text);

void main() {
  print(parseMark('75'));
  print(parseMark('7S'));
}

75
null

#Question 6 — Solution

void main() {
  const cardNumber = '4111222233334444';
  final lastFour = cardNumber.substring(cardNumber.length - 4);
  print('Card: **** $lastFour');
}

Card: **** 4444

#Question 7 — Solution

(a) blocking — a security problem. (b) nit. (c) suggestion, and the author may fairly decline.

#Question 8 — Solution

Unclear names, an index loop, no braces, and NaN for an empty list.

double? averageTemperature(List<double> temperatures) {
  if (temperatures.isEmpty) return null;
  var sum = 0.0;
  for (final temperature in temperatures) {
    sum += temperature;
  }
  return sum / temperatures.length;
}

void main() {
  print(averageTemperature([20.0, 25.0]));
  print(averageTemperature([]));
}

22.5
null

#Question 9 — Solution

"Thanks for looking closely! This runs once per report on three items, so I've kept it simple for now. Happy to change it if the list grows."

#Question 10 — Solution

import 'package:class_report/grades.dart';
import 'package:test/test.dart';

void main() {
  test('80 is the lowest A', () {
    expect(gradeFor(80), 'A');
    expect(gradeFor(79.9), 'B');
  });
  test('50 is the lowest B', () {
    expect(gradeFor(50), 'B');
    expect(gradeFor(49.9), 'C');
  });
}

00:00 +2: All tests passed!

#Quick Revision

1. What is a code review?
2. What should you do before asking someone to review your code?
3. Why is ! on a map lookup risky?
4. What does an empty catch block hide?
5. What do the labels blocking, suggestion and nit mean?
6. How should you respond to a comment you disagree with?

#Key Takeaways

• A code review is a second person reading a change before it ships.
• Review yourself first: dart format, dart analyze, dart test, then read your diff.
• Tools check that code is well formed; reviewers check that it is right.
• Checklist: correctness, edge cases, null safety, errors, naming, tests, tools, performance, security.
• Every ! needs a reason; a null check is usually better.
• Never swallow exceptions; never print secrets.
• Good comments are kind, specific, explain why, suggest a fix and carry a label.
• Receive feedback as being about the code, reply to every comment, and say thanks.

#What's Next?

Lecture 128 — Dart Language Evolution & Versioning. Category 22 looks at modern Dart. We start with how the language itself changes: release versions, language versions, and how the SDK constraint in pubspec.yaml decides which features your code can use.