-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add: Example usage to fillRange method. #37313
Add: Example usage to fillRange method. #37313
Conversation
@lrhn ? |
… the error that is raised during operation.
@kevmoo Added one more commit which updates the insert method documentation. |
Reviewing in https://dart-review.googlesource.com/c/sdk/+/106724 |
@@ -674,6 +674,22 @@ abstract class List<E> implements EfficientLengthIterable<E> { | |||
* A range from [start] to [end] is valid if `0 <= start <= end <= len`, where | |||
* `len` is this list's `length`. The range starts at `start` and has length | |||
* `end - start`. An empty range (with `end == start`) is valid. | |||
* |
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.
Just for future reference: You can use the code-block syntax:
```dart
List<int> list1 = ...
etc...
```
It's easier to maintain than the old "four-space-indent" for code blocks, so I'd prefer that for new comments even if it's inconsistent with the surrounding code.
(No need to change the code already written, though, so this is fine).
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.
Noted
sdk/lib/core/list.dart
Outdated
@@ -464,7 +464,7 @@ abstract class List<E> implements EfficientLengthIterable<E> { | |||
* This increases the length of the list by one and shifts all objects | |||
* at or after the index towards the end of the list. | |||
* | |||
* An error occurs if the [index] is less than 0 or greater than length. | |||
* A `RangeError` occurs if the [index] is less than 0 or greater than length. |
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'd prefer to not say whic error is throws. Instead replace this entire section by:
* The list must be growable.
* The [index] value must be non-negative and no greater than [length].
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.
awesome
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 this applies to insertAll
method as well. I am updating it for that as well.
sdk/lib/core/list.dart
Outdated
@@ -674,6 +674,22 @@ abstract class List<E> implements EfficientLengthIterable<E> { | |||
* A range from [start] to [end] is valid if `0 <= start <= end <= len`, where | |||
* `len` is this list's `length`. The range starts at `start` and has length | |||
* `end - start`. An empty range (with `end == start`) is valid. | |||
* | |||
* List<int> list1 = new List(3); |
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'd start with:
*
* Example:
to set the expectation.
Also, change list1
to just list
, the "1" doesn't add anything to the example.
sdk/lib/core/list.dart
Outdated
* | ||
* List<int> list = new List(); | ||
* list.fillRange(0, 2, 1); // throws `RangeError: Invalid Value` exception. | ||
* |
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 is an example of what not to do, which we try not to do (unless the error is particularly interesting, this one isn't).
I'd just drop this example.
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 added this keeping in mind it as an extra note or gotcha while using the fillRange method. But with your suggested comment:
The list must be growable.
It give user a sense that using this method might result in some error.
I will go ahead and remove this example.
sdk/lib/core/list.dart
Outdated
* | ||
* List<int> list2 = new List()..length = 3; | ||
* list2.fillRange(0, 2, 1); | ||
* print(list2); // [1, 1, null] |
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 example is exactly the same as the first one, except that the list is growable. Since this function doesn't care about being growable, the example is not adding anything new. I'd remove this example too.
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.
okay
Change: documentation for `insert` and `insertAll` method.
Thank for contributing @abhishekkanojia |
No description provided.