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

C-CONV and C-CONV-TRAITS do not mention each other and are unclear

Open orlp opened this issue 7 years ago • 1 comments

C-CONV and C-CONV-TRAITS do not mention each other as possibilities, while both are very closely related (in providing conversions), and as an API designer you might want to choose between them or implement multiple.

Furthermore, C-CONV-TRAITS is exceptionally unclear:

The following conversion traits should be implemented where it makes sense:

If it made sense when to implement these to the reader they wouldn't be reading the guideline in the first place. There should be a clear description of when to implement these traits.

orlp avatar Jul 10 '18 20:07 orlp

We should try be more direct on where it makes sense. My thoughts are:

  • From: For infallible generic conversions. If you have a type T that you might want to pass to a method that accepts a impl Into<U>, then consider implementing From<T> for U.
  • TryFrom: For fallible generic conversions. If you have a type T that you might want to pass to a method that accepts a impl TryInto<U>, then consider implementing TryFrom<T> for U. (Although, I've never written impl TryInto myself).
  • AsRef: For by-ref generic conversions (probably avoid relying solely on AsRef because in common use it can interact poorly with inference, so we have str::as_bytes as well as impl AsRef<[u8]> for str). If you have a type T that you might want to pass to a method that accepts a impl AsRef<U> then consider implementing AsRef<U> for T.
  • AsMut: For by-mut generic conversions (probably avoid relying solely on AsMut because in common use it can interact poorly with inference, so we have Vec<T>::as_mut_slice as well as impl AsMut<[T]> for Vec<T>.

We also don't mention the Borrow trait, which on the surface seems like AsRef but has a few differences in its blanket implementations, relationship with ToOwned and semantics with an equivalence relationship between T and T::Owned.

KodrAus avatar Dec 22 '20 04:12 KodrAus