-
-
Notifications
You must be signed in to change notification settings - Fork 178
Simplification: No longer return "impl Iterator" #619
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
Simplification: No longer return "impl Iterator" #619
Conversation
05d2ddb
to
efe840b
Compare
efe840b
to
ad6c5a7
Compare
What does @nicholasbishop think?
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.
lgtm. I only recently became aware that returning an impl SomeTrait
can be kind of an API footgun, especially with a trait like Iterator
where you often actually want to expose additional traits at the same time.
I do not see a benefit in the impl Iterator design pattern in our code base. Using a specific, public type is simpler for end-users
Agreed. When I initially started using the impl trait
pattern in the code, it was fairly recently stabilised, and it seemed pretty useful since it "simplified" the public API by hiding away irrelevant structures, which were only defined to implement some trait or another. I didn't realize at the time that there would be downsides to this approach.
Uh oh!
There was an error while loading. Please reload this page.
This commit addresses #616 and replaces three occurrences of
impl Iterator
with a specific type. I do not see a benefit in theimpl Iterator
design pattern in our code base. Using a specific, public type is simpler for end-users, as discussed in #616.impl Something
only makes sense, I think, in use-cases where users specify code, such as inactix-web
:Or in situations where the return type is complex and nested with many generics.
Checklist