api-guidelines icon indicating copy to clipboard operation
api-guidelines copied to clipboard

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

Open chris-morgan opened this issue 7 years ago • 1 comments

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.)

chris-morgan avatar Nov 08 '17 10:11 chris-morgan

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 avatar Dec 22 '20 02:12 KodrAus