-
Notifications
You must be signed in to change notification settings - Fork 356
Polish SafeAccess extension methods to use getters and support Iterable. #1647
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
Conversation
87fdcc0
to
0f1630f
Compare
Fyi filed |
} | ||
|
||
extension SafeAccess<T> on Iterable<T> { | ||
T get safeFirst { |
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:
T get safeFirst => isNotEmpty ? first : null;
T get safeLast => isNotEmpty ? last : 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.
done. also made safeGet a one liner.
final list = []; | ||
expect(list.safeFirst(), isNull); | ||
final list = <int>[]; | ||
final Iterable<int> iterable = list; |
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: Do we need the Iterable on lvalue with list = [] inference e.g.,
Iterable iterable = list; ?
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 cast to I do want to test on Iterable to make sure the extension methods are applying to Iterable as well as List. Maybe that doesn't need to be tested but i think it is a reasonable sort of thing to verify given that extension method behavior does vary depending on the type of the target.
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.
Few nits - lgtm
No description provided.