-
Notifications
You must be signed in to change notification settings - Fork 715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Constructors page #5757
Conversation
Visit the preview URL for this PR (updated for commit 22ed8d3): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few re-orderings suggested. I think these should make the doc flow fairly well, but I'm having a hard time landing on a clearly optimal way to lay it out since so many of the details are intermingled.
src/content/language/constructors.md
Outdated
## Parameter initialization | ||
|
||
Dart can initialize parameters in a constructor in three ways. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to call this section ## Instance Variable Initialization
.
The "parameter" name refers to when it's a constructor argument. This section covers initialization of the class fields.
Also, I'm iffy on the phrasing "in a constructor" here, since the first way "when declaring variables" is outside of the constructor.
## Parameter initialization | |
Dart can initialize parameters in a constructor in three ways. | |
## Instance Variable Initialization | |
Dart can initialize instance variables in three ways. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
src/content/language/constructors.md
Outdated
### Initialize parameters when declaring variables | ||
|
||
Initialize the constructor parameters when you declare variables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
### Initialize parameters when declaring variables | |
Initialize the constructor parameters when you declare variables. | |
### Initialize instance variables in the declaration | |
Initialize the instance variables in the variable declaration. |
src/content/language/constructors.md
Outdated
// The parameterless constructor is not even be needed to set to (1.0,2.0) | ||
PointA(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about showing the constructor commented out to reinforce that it is implicit and does not need to be defined?
// The parameterless constructor is not even be needed to set to (1.0,2.0) | |
PointA(); | |
// The default (parameterless) constructor is implicit and will set to (1.0,2.0) | |
// PointA(); |
src/content/language/constructors.md
Outdated
* To declare a constructor, create a function named the same as its class. | ||
You can add an optional additional identifier as described in | ||
[Named constructors](#named-constructors). | ||
|
||
Use the most common constructor, the generative constructor, to create a new | ||
instance of a class, and [initializing formal parameters](#initializing-formal-parameters) | ||
to instantiate any instance variables, if necessary: | ||
* To instantiate a class, use the | ||
[generative constructor](#generative-constructors). | ||
|
||
<?code-excerpt "misc/lib/language_tour/classes/point_alt.dart (idiomatic-constructor)" plaster="none"?> | ||
```dart | ||
class Point { | ||
double x = 0; | ||
double y = 0; | ||
* To instantiate any instance variables, | ||
[initialize formal parameters](#parameter-initialization). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third bullet point should refer to "initialize instance variables" instead of "initialize forma parameters". I think it also reads a little awkwardly with this bullet list pattern. What do you think of rephrasing as general facts about constructors instead of fitting into the "to <> use <>" pattern?
- A constructor is a function with the same name as its class, with an optional additional identifier as described in
Named constructors
. - A constructor may be a factory, or forward to another implementation. The constructors which instantiate the class are
generative constructors
. - The generative constructor will
initialize instance variables
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored with explanatory list.
src/content/language/constructors.md
Outdated
::: | ||
## Types of constructors | ||
|
||
### Constructor inheritance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this a level 2 header and move this section after the last subheader of the Types of constructors
section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can do, though I think this has info relevant to the succeeding descriptions of types of constructors. Let's see how it reads.
src/content/language/constructors.md
Outdated
```dart | ||
// If you invoke the super constructor (`super(0)`) with any | ||
// positional arguments, using a super parameter (`super.x`) | ||
// returns an error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] I don't know what "returns" would mean in this context. How about "is an error" or "results in an error"?
// returns an error. | |
// is an error. |
} | ||
``` | ||
|
||
Constant constructors don't always create constants. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[optional]
Constant constructors don't always create constants. | |
Constant constructors may be invoked in a non-const context and don't always create constants. |
[using constructors][]. | ||
|
||
When encountering one of two cases of implementing a constructor, | ||
use the `factory` keyword: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] Is this missing a word at the end? use the factory keyword when:
? use the factory keyword if:
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just leads into a bulleted list of when you would use a factory.
src/content/language/constructors.md
Outdated
// Initializing formal parameters can also be optional. | ||
PointB.optional([this.x = 0.0, this.y = 0.0]); | ||
PointB.named({required this.x, required this.y}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the current layout of the doc the last line is covered well in PointC
- I think it flows well now to omit the mention here. The optional positional parameters are still useful to show here.
// Initializing formal parameters can also be optional. | |
PointB.optional([this.x = 0.0, this.y = 0.0]); | |
PointB.named({required this.x, required this.y}); | |
} | |
// Initializing formal parameters can also be optional. | |
PointB.optional([this.x = 0.0, this.y = 0.0]); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, but then we should "name" PointC
constructors to illustrate the point.
} | ||
``` | ||
|
||
This also works with named variables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should mention named variables before private fields. WDYT about flipping them - move lines 426-442 to after the example currently ending on line 464
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. Let me commit these changes first and you can have another look.
Do we have the capability to create a link to another header inside a code block? (I expect the answer is no) I think having some kind of syntax "glossary" early in the doc could help? class Super {
final String field1; // Instance variable declaration
final String field2 =
someExpression(); // Instance variable declaration with initializing expression
Super() // Unnamed generative constructor
: field1 = 'default'; // Initializer list
Super.named(String argument) // Named generative constructor
: field1 = argument;
Super.initializingFormal(this.field1); // Initializing formal parameter
factory Super.factory() => Super.named('from factory'); // Factory constructor
Super.redirecting() // Redirecting constructor
: this.named('from redirecting');
}
class Foo extends Super {
Foo()
: super.named('foo unnamed constructor'); // Super constructor invocation
Foo.named(super.argument)
// Super parameter
: super.named();
} |
PTAL @natebosch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played around with the layout this afternoon.
I think it we're pretty close but there are a few things I'd change.
- Move
Non-default superclass constructors
to nest underConstructor Inheritance
. - Move
Super parameters
up one level - nested underConstructor Inhericance
as a sibling toNon-default superclass constructors
. - Move
Instance Variable Initialization
beforeConstructor Inheritance
.
## Types of Constructors
### // all current, except Non-default superclass constructors
## Instance Variable Initialization
### // all current
## Constructor Inheritance
### Non-default superclass constructors
### Super Parameters
I don't think we need much else changed after that. I had started on some changes in the intro, but in the end I didn't find anything I liked better than whats here.
Edit: although we might need to double check if the reorder between super-parameters and instance variable initialization sections caused any problems.
Fixes #2009
Fixes #2221
Fixes #5227
Fixes #5506
Fixes #5728
Fixes #5738
Fixes #5739