vortex icon indicating copy to clipboard operation
vortex copied to clipboard

Make IntoArrayVariant chain more explicit

Open a10y opened this issue 1 year ago • 2 comments

Currently we implement IntoArrayVariant for all T: IntoCanonical by first calling into_canonical and then downcasting. This hides that there is a potentially expensive decoding step going on.

We should either remove the shortcut method, or rename the methods to make it clearer that they can be expensive

a10y avatar Jul 24 '24 12:07 a10y

Strong preference for single methods to avoid long chains. Need a good name though and likely will deviate from rust conventions

robert3005 avatar Jul 24 '24 13:07 robert3005

some options:

  • canonicalize_to_primitive
  • into_canonical_primitive
  • to_canonical_primitive
  • to_primitive

Another option is to replace methods with top-level function calls, e.g. canonical::to_primitive(array)

a10y avatar Jul 24 '24 18:07 a10y

I think into/to_canonical_primitive is likely what I would go for. Top level functions would also make it more explicit. Would try the rename and otherwise make them separate functions

robert3005 avatar Oct 23 '24 09:10 robert3005