Skip to content
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

Merged
merged 9 commits into from
May 7, 2024
Merged

Refactor Constructors page #5757

merged 9 commits into from
May 7, 2024

Conversation

atsansone
Copy link
Contributor

@atsansone atsansone commented Apr 26, 2024

Fixes #2009
Fixes #2221
Fixes #5227
Fixes #5506
Fixes #5728
Fixes #5738
Fixes #5739

@dart-github-bot
Copy link
Collaborator

dart-github-bot commented Apr 26, 2024

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

https://dart-dev--pr5757-fix-5728-hns4q3qb.web.app

Copy link
Member

@natebosch natebosch left a 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.

Comment on lines 362 to 364
## Parameter initialization

Dart can initialize parameters in a constructor in three ways.
Copy link
Member

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.

Suggested change
## Parameter initialization
Dart can initialize parameters in a constructor in three ways.
## Instance Variable Initialization
Dart can initialize instance variables in three ways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment on lines 366 to 368
### Initialize parameters when declaring variables

Initialize the constructor parameters when you declare variables.
Copy link
Member

Choose a reason for hiding this comment

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

How about

Suggested change
### 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.

Comment on lines 376 to 377
// The parameterless constructor is not even be needed to set to (1.0,2.0)
PointA();
Copy link
Member

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?

Suggested change
// 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();

Comment on lines 15 to 23
* 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).
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored with explanatory list.

:::
## Types of constructors

### Constructor inheritance
Copy link
Member

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?

Copy link
Contributor Author

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.

```dart
// If you invoke the super constructor (`super(0)`) with any
// positional arguments, using a super parameter (`super.x`)
// returns an error.
Copy link
Member

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"?

Suggested change
// returns an error.
// is an error.

}
```

Constant constructors don't always create constants.
Copy link
Member

Choose a reason for hiding this comment

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

[optional]

Suggested change
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:
Copy link
Member

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: ?

Copy link
Contributor Author

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.

Comment on lines 420 to 285
// 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});
}
Copy link
Member

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.

Suggested change
// 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]);
}

Copy link
Contributor Author

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.
Copy link
Member

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

Copy link
Contributor Author

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.

@natebosch
Copy link
Member

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();
}

@atsansone
Copy link
Contributor Author

PTAL @natebosch

@parlough parlough requested a review from natebosch May 6, 2024 23:48
Copy link
Member

@natebosch natebosch left a 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 under Constructor Inheritance.
  • Move Super parameters up one level - nested under Constructor Inhericance as a sibling to Non-default superclass constructors.
  • Move Instance Variable Initialization before Constructor 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.

@atsansone atsansone merged commit 84c218b into dart-lang:main May 7, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment