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

C-NEWTYPE-HIDE neglects to mention other traits implemented by the wrapped type #142

Open
chris-morgan opened this issue Nov 8, 2017 · 1 comment
Labels
accepted An amendment that's been accepted and can be applied amendment Amendments to existing guidelines

Comments

@chris-morgan
Copy link
Member

C-NEWTYPE-HIDE neglects to mention something that I feel is quite important:

Enumerate<Skip<I>> does not just implement Iterator; it may also implement some or all of Debug, Clone, FusedIterator, ExactSizeIterator and DoubleEndedIterator.

By wrapping the Enumerate<Skip<I>> in a newtype, you lose those implementations. (This is also my biggest concern with practical usage of impl Trait, though I definitely want it.)

Thus, I say that for private code, wrapping things in a newtype like that is a mild anti-pattern as it fights against various optimisations and use cases. (Use a type alias, by all means; approximately all the benefits without any of the maintenance cost.)

For public code, wrapping such implementation details that could conceivably change in a newtype is the responsible and elegant thing to do, but the guideline needs to mention this detail about other traits, and that you should also add to your code most or all of these—

#[derive(Clone, Debug)]

impl<I: FusedIterator> FusedIterator for MyTransformResult<I> {}
impl<I: ExactSizeIterator> ExactSizeIterator for MyTransformResult<I> {}
impl<I: DoubleEndedIterator + ExactSizeIterator> DoubleEndedIterator<I> for MyTransformResult<I> {}

(While we’re here: the Result suffix in MyTransformResult should be killed with prejudice.)

@KodrAus KodrAus added the amendment Amendments to existing guidelines label Dec 22, 2020
@KodrAus
Copy link
Contributor

KodrAus commented Dec 22, 2020

We could link to other guidelines suggesting appropriate traits are implemented from this one, which I think should cover ensuring a newtype is appropriately usable.

@KodrAus KodrAus added the accepted An amendment that's been accepted and can be applied label Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted An amendment that's been accepted and can be applied amendment Amendments to existing guidelines
Projects
None yet
Development

No branches or pull requests

2 participants