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

RFC: standardizing on naming functions that convert from one type to another #7087

Closed
erickt opened this Issue Jun 12, 2013 · 15 comments

Comments

Projects
None yet
10 participants
@erickt
Contributor

erickt commented Jun 12, 2013

It's time to handle another one of the hardest things in computer science, how to name conversion functions. Rust supports essentially three ways of converting from one type to another. First there is doing a copy, such as these in the str mod:

pub fn from_bytes(vv: &[u8]) -> ~str { ... }

pub trait StrSlice<'self> {
     fn to_str(&self) -> ~str { ... }
     fn to_owned(&self) -> ~str { ...}
}

Then we have some that can do a no-allocation moves, such as either's:

pub fn to_result<T, U>(eith: Either<T, U>) -> Result<U, T> { ... }

Finally, we have ones that can get a no-copy reference, as in str's:

pub fn from_bytes_with_null<'a>(vv: &'a [u8]) -> &'a str { ... }

pub trait StrSlice<'self> {
    fn as_bytes(&self) -> &'self [u8]
}

While they share similar names, it's not consistent what exactly is happening in each case. I would like to standardize on a style so I can know how to name things for the standard library.

Some options that came up in IRC are:

  • as for conversions, and to for moves or copies. Simple, but then we end up with functions named like to_str_consume to distinguish between the two cases, which is a little ugly.
  • as for conversions, to for moves, and into for copies. This is nice in that we try to push users to use our no-copy solutions, but this will make .to_str a little more ugly.
  • as for conversions, to for copies, and into for moves. This keeps .to_str pretty, but people might tend to the copy functions instead of the moves.
  • Use a standard trait for conversions, as in #7080. This won't help when we have options for how to convert. For example, from_bytes_slice<'a>(vector: &'a [u8]) -> &'a str which handles byte slices that don't end with a null character, or str::from_bytes_with_null<'a>(vv: &'a [u8]) -> &'a str which does.

Anyone have an opinion on what we should use?

@bstrie

This comment has been minimized.

Show comment
Hide comment
@bstrie

bstrie Jun 17, 2013

Contributor

This is an important consideration to keep us all sane and free from constant naming squabbles.

I don't think there's really much to debate here. as for conversions is obvious since that's our casting keyword. And seeing as how we keep steadily creeping towards explicitness and making allocations apparent, into for copying and to for moving seem appropriate. If it makes .to_str() two characters more verbose, then good!

Contributor

bstrie commented Jun 17, 2013

This is an important consideration to keep us all sane and free from constant naming squabbles.

I don't think there's really much to debate here. as for conversions is obvious since that's our casting keyword. And seeing as how we keep steadily creeping towards explicitness and making allocations apparent, into for copying and to for moving seem appropriate. If it makes .to_str() two characters more verbose, then good!

@Kimundi

This comment has been minimized.

Show comment
Hide comment
@Kimundi

Kimundi Jun 17, 2013

Member

Hm, on the one hand using the short prefixes for cheap operations makes sense...
On the other hand into_str()...

Plus "into" doesn't really make sense as a prefix for doing a copy, I think.
How about:

be_str() - move (be(come) a str)
as_str() - ref (refer-to as str)
to_str() - copy (copy to a str)
Member

Kimundi commented Jun 17, 2013

Hm, on the one hand using the short prefixes for cheap operations makes sense...
On the other hand into_str()...

Plus "into" doesn't really make sense as a prefix for doing a copy, I think.
How about:

be_str() - move (be(come) a str)
as_str() - ref (refer-to as str)
to_str() - copy (copy to a str)
@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jul 10, 2013

Contributor

nominating

Contributor

brson commented Jul 10, 2013

nominating

@Thiez

This comment has been minimized.

Show comment
Hide comment
@Thiez

Thiez Jul 10, 2013

Contributor

Could this be done with some traits?

trait MoveConvert<T> {
  fn be(self)->T;
}
trait RefConvert<T> {
  fn as(&self)->&T;
}
trait CopyConvert<T> {
  fn to(&self)->T;
}

Of course the 'as' method would need another name as it is currently in use as a keyword.

Contributor

Thiez commented Jul 10, 2013

Could this be done with some traits?

trait MoveConvert<T> {
  fn be(self)->T;
}
trait RefConvert<T> {
  fn as(&self)->&T;
}
trait CopyConvert<T> {
  fn to(&self)->T;
}

Of course the 'as' method would need another name as it is currently in use as a keyword.

@erickt

This comment has been minimized.

Show comment
Hide comment
@erickt

erickt Jul 10, 2013

Contributor

@Thiez, yeah, I talk about that in #7080.

Contributor

erickt commented Jul 10, 2013

@Thiez, yeah, I talk about that in #7080.

@Thiez

This comment has been minimized.

Show comment
Hide comment
@Thiez

Thiez Jul 10, 2013

Contributor

My apologies, I totally missed that even though it is in the original issue description. I suppose you can count my opinion as one in support of that particular solution.

Contributor

Thiez commented Jul 10, 2013

My apologies, I totally missed that even though it is in the original issue description. I suppose you can count my opinion as one in support of that particular solution.

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jul 31, 2013

Contributor

I'd like to get this and other stylistic issues that affect std resolved this year and written up as coding standards (better than https://github.com/mozilla/rust/wiki/Note-style-guide)

Contributor

brson commented Jul 31, 2013

I'd like to get this and other stylistic issues that affect std resolved this year and written up as coding standards (better than https://github.com/mozilla/rust/wiki/Note-style-guide)

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Sep 5, 2013

Contributor

accepted for backwards-compatible

Contributor

catamorphism commented Sep 5, 2013

accepted for backwards-compatible

@erickt

This comment has been minimized.

Show comment
Hide comment
@erickt

erickt Sep 12, 2013

Contributor

Another bikeshed option. We could rename methods like .to_foo(&self) -> Foo, to .make_foo(&self) -> Foo. It's not that much longer, and make_ suggests to me that something is being constructed. This would free us up to use the shorter .to_foo(self) -> Foo form for moving values.

Contributor

erickt commented Sep 12, 2013

Another bikeshed option. We could rename methods like .to_foo(&self) -> Foo, to .make_foo(&self) -> Foo. It's not that much longer, and make_ suggests to me that something is being constructed. This would free us up to use the shorter .to_foo(self) -> Foo form for moving values.

@Kimundi

This comment has been minimized.

Show comment
Hide comment
@Kimundi

Kimundi Sep 12, 2013

Member

It seems the codebase is slowly moving into the to/as/into direction as proposed in #7151. At least a few very common operations on, eg, vectors now use this convention.

However, the problem is that to and into suggest the wrong order of precedence: You should always move if possible.

If be for move and to for reference-copy doesn't get accepted for being too confusing/similar names, then I'd like to propose yet another alternative naming convention (yaanc):

  • to_TYPE for a self-by-value conversion, which would usually imply no new allocation and a move.
  • as_TYPE for a reference/slice conversion.
  • refto_TYPE for a conversion of the value done by reference, usually implying a new allocation.

The main problem is that the current to_TYPE is very common, so switching it to refto_TYPE would probably result in some resistance:

let v = [1_u8, 2, 3];
let s = v.refto_str();
let v2 = v.refto_owned();

Regardless of concrete name, we should also make this a guideline:
If such conversion methods need to be implemented, at least to and as should exists, with refto being optional in cases where

  • the operation can be faster than value.clone().to_TYPE,
  • implementing Clone is not possible,
  • or the refto operation is common enough that providing a shortcut makes sense.
Member

Kimundi commented Sep 12, 2013

It seems the codebase is slowly moving into the to/as/into direction as proposed in #7151. At least a few very common operations on, eg, vectors now use this convention.

However, the problem is that to and into suggest the wrong order of precedence: You should always move if possible.

If be for move and to for reference-copy doesn't get accepted for being too confusing/similar names, then I'd like to propose yet another alternative naming convention (yaanc):

  • to_TYPE for a self-by-value conversion, which would usually imply no new allocation and a move.
  • as_TYPE for a reference/slice conversion.
  • refto_TYPE for a conversion of the value done by reference, usually implying a new allocation.

The main problem is that the current to_TYPE is very common, so switching it to refto_TYPE would probably result in some resistance:

let v = [1_u8, 2, 3];
let s = v.refto_str();
let v2 = v.refto_owned();

Regardless of concrete name, we should also make this a guideline:
If such conversion methods need to be implemented, at least to and as should exists, with refto being optional in cases where

  • the operation can be faster than value.clone().to_TYPE,
  • implementing Clone is not possible,
  • or the refto operation is common enough that providing a shortcut makes sense.
@cpeterso

This comment has been minimized.

Show comment
Hide comment
@cpeterso

cpeterso Sep 17, 2013

Contributor

One more bikeshed option: new prefix for copy functions like .new_foo(&self) -> Foo. new is a familiar programming term, suggests something is being constructed, and is one letter shorter than into and make. ;)

Contributor

cpeterso commented Sep 17, 2013

One more bikeshed option: new prefix for copy functions like .new_foo(&self) -> Foo. new is a familiar programming term, suggests something is being constructed, and is one letter shorter than into and make. ;)

@huonw

This comment has been minimized.

Show comment
Hide comment
@huonw

huonw Sep 17, 2013

Member

We should probably also decide on something for (e.g.) intuint. i.e. a very cheap conversion that isn't really an as (i.e. no lifetimes/not a reference conversion), but doesn't have the same performance consequences as to normally does.

Currently we're just using to.

Member

huonw commented Sep 17, 2013

We should probably also decide on something for (e.g.) intuint. i.e. a very cheap conversion that isn't really an as (i.e. no lifetimes/not a reference conversion), but doesn't have the same performance consequences as to normally does.

Currently we're just using to.

@Kimundi

This comment has been minimized.

Show comment
Hide comment
@Kimundi

Kimundi Sep 17, 2013

Member

@huonw I don't think that should be relevant. it makes an copy, whether that's actually expensive depends on the type.

Member

Kimundi commented Sep 17, 2013

@huonw I don't think that should be relevant. it makes an copy, whether that's actually expensive depends on the type.

@errordeveloper

This comment has been minimized.

Show comment
Hide comment
@errordeveloper

errordeveloper Jul 7, 2014

Contributor

This was opened a year ago and been last updated more then 6 month ago, is it still outstanding?

Contributor

errordeveloper commented Jul 7, 2014

This was opened a year ago and been last updated more then 6 month ago, is it still outstanding?

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon Aug 4, 2014

Member

The guidelines have now made as/to/into official. I believe that most of the libraries already adhere to this policy, but we will be on the lookout during API stabilization.

Closing as resolved.

Member

aturon commented Aug 4, 2014

The guidelines have now made as/to/into official. I believe that most of the libraries already adhere to this policy, but we will be on the lookout during API stabilization.

Closing as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment