Skip to content

Split OOP tutorial chapter and improve progressive disclosure - #7506

Open
conooi wants to merge 4 commits into
dart-lang:mainfrom
conooi:tutorial-fix-p3
Open

Split OOP tutorial chapter and improve progressive disclosure#7506
conooi wants to merge 4 commits into
dart-lang:mainfrom
conooi:tutorial-fix-p3

Conversation

@conooi

@conooi conooi commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Resolves cognitive overload and uneven pacing in the tutorial's OOP content by splitting the monolithic OOP chapter into two progressive lessons and deconstructing complex classes into incremental steps.

Changes

  • Split OOP into two chapters: Chapter 5 (Classes and objects) focuses on core classes, constructors, getters, and enums; Chapter 6 (Inheritance and abstract classes) introduces abstract classes, extends, @override, and command execution.
  • Improved progressive disclosure: Deconstructs Option and Command step-by-step; explains Dart features like implicit getters satisfying abstract getters, late, UnmodifiableSetView, and records.
  • Maintained continuity: Synchronized navigation (tutorial.yml, getStarted.yml), separated quizzes, and renumbered downstream chapter prerequisites (Chapters 7–13).
  • Verified code flow: Tested end-to-end in a clean project with dart analyze (0 issues) and dart run.

Fixes #7333
Fixes #7418

@conooi
conooi requested a review from parlough September 12, 2026 01:13

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request restructures the Dart CLI tutorial by splitting the Object-Oriented Programming chapter into two distinct lessons: "Define classes and objects" and "Structure apps with inheritance and abstract classes". This change involves updating prerequisite chapter references across multiple tutorial files, adding a new quiz for inheritance, and updating navigation files. The review feedback highlights several performance and idiomatic improvements in the newly added code blocks, such as avoiding redundant set allocations in getters by using Set or Iterable directly, and removing the unnecessary async keyword from synchronous methods returning FutureOr.

Comment on lines +223 to +226
final List<Option> _options = [];

UnmodifiableSetView<Option> get options =>
UnmodifiableSetView(_options.toSet());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Defining _options as a List and then converting it to a Set via _options.toSet() on every access of the options getter is inefficient because it allocates a new Set and a new UnmodifiableSetView every time the getter is read. Additionally, it loses the order of options if they are converted to a standard unordered set.

Instead, define _options as a Set<Option> internally. This naturally prevents duplicate options from being added to the same command, and allows wrapping it directly in UnmodifiableSetView without copying or reallocating on every read.

Suggested change
final List<Option> _options = [];
UnmodifiableSetView<Option> get options =>
UnmodifiableSetView(_options.toSet());
final Set<Option> _options = {};
UnmodifiableSetView<Option> get options =>
UnmodifiableSetView(_options);

Comment on lines +329 to +330
UnmodifiableSetView<Command> get commands =>
UnmodifiableSetView<Command>(<Command>{..._commands.values});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Exposing commands by creating a new Set and wrapping it in UnmodifiableSetView on every getter access is inefficient because it performs a copy-on-read. Since the caller only needs to iterate over the commands (e.g., in the HelpCommand loop), it is much simpler and more efficient to expose them as an Iterable<Command> directly from the map's values. This avoids any extra allocations or imports.

Suggested change
UnmodifiableSetView<Command> get commands =>
UnmodifiableSetView<Command>(<Command>{..._commands.values});
Iterable<Command> get commands => _commands.values;

Comment on lines +420 to +428
FutureOr<Object?> run(ArgResults args) async {
var usage = runner.usage;
for (var command in runner.commands) {
usage += '\n ${command.usage}';
}

return usage;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The run method is marked as async but does not use the await keyword anywhere in its body. Since the method returns FutureOr<Object?>, it can return the computed string synchronously without the overhead of wrapping it in a Future and scheduling it on the event loop. Removing the async keyword is more efficient and idiomatic.

Suggested change
FutureOr<Object?> run(ArgResults args) async {
var usage = runner.usage;
for (var command in runner.commands) {
usage += '\n ${command.usage}';
}
return usage;
}
}
@override
FutureOr<Object?> run(ArgResults args) {
var usage = runner.usage;
for (var command in runner.commands) {
usage += '\n ${command.usage}';
}
return usage;
}

@dart-github-bot

Copy link
Copy Markdown
Collaborator

Visit the preview URL for this PR (updated for commit a0c3134):

https://dart-dev--pr7506-tutorial-fix-p3-rhmae8yr.web.app

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uneven level of information You are pushing user instantly to very deep water starting from this lesson.

2 participants