Rust's suggested API guidelines for conversion method naming

[TheDan64]

Although not required, rust's API best practice guideline suggests conversion guidelines for as_*, to_*, and into_* methods.

So, I think this would look like:

fn into_foo_bar(self) -> FooBar { /* panic on not FooBar */ };
fn as_foo_bar(&self) -> Option<&FooBar> { ... };

Not sure about to, maybe that's just taking a borrowed Clone type and cloning it? And copying a Copy? Basically like into but taking a &self?

fn to_foo_bar(&self) -> FooBar { /* panic on not FooBar */ };

But looking at the str::to_owned() -> String maybe it should instead be FooBar.to_foo_bar_enum?

What do you think?

[alekratz]

I think that into_* and as_* methods would be the right way to go. Good call on that.

As far as the to_* method goes, my main concern is that we can't detect if the enum is Clone, Copy, or neither. So we would probably have to separate this behavior into a new custom derive, e.g. EnumCloneGetters. Since all Copy types are Clone types, I think it would be okay to skip implementation of a derive specifically for Copy types.

[alekratz]

Also, just a related thought, if we're going to be doing things by naming convention, would it make sense to update the derive names to match? EnumIntoGetters, EnumAsGetters, etc? And then EnumGetters could be a shortcut just to implement both.

[TheDan64]

Naming them that way sounds like a great idea.

[alekratz]

Well, if you want to add this, go for it. Otherwise, I'll probably add it this weekend.

[TheDan64]

Not sure I'll be able to get to it this weekend so it's all yours!

I was thinking - maybe it's best to not have the to_* methods for now? There seems to be a less obvious and straightforward use-case compared to into_* and as_* methods. Maybe add it later if someone really needs a Clone version? Or split it into another issue for further review/decision making?

[alekratz]

I'm fine with leaving to_* off the table for now. Go ahead and make another issue for further review.

[alekratz]

A conflict occurs: EnumGetters currently implements special getters for known¹ Copy types, which will just copy the type.

I see a few options:

• Keep EnumGetters as it is and add EnumAsGetters and EnumIntoGetters
• Remove EnumGetters and remove copying behavior completely
• Remove EnumGetters and transfer copying behavior to EnumAsGetters (i.e. as_foo() where foo is a known copy type would return Option<T> instead of Option<&T>).

After typing this out, I'm leaning towards the second option.

¹see rust-lang/rust#25893 for more details regarding "known" copy types

[TheDan64]

Well, into methods should be moves, so copy shouldn't matter there right? as methods shouldn't have to worry about copy/clone either if it returns an `Option<&T>`. Because the user can get a reference to the inner data, and if it so happens it is copy or clone, then they can take advantage of that naturally. IE `*foobar.as_foo().unwrap()` should copy automatically if you try and move it and it is a copy type and similarly `foobar.as_foo().unwrap().clone()` should work if the inner data happens to be clone? ⁣

…

On Jul 22, 2017, 12:14 PM, at 12:14 PM, Alek Ratzloff ***@***.***> wrote: A conflict occurs: `EnumGetters` currently implements special getters for known¹ `Copy` types, which will just copy the type. I see a few options: * Keep `EnumGetters` as it is and add `EnumAsGetters` and `EnumIntoGetters` * Remove `EnumGetters` and remove copying behavior completely * Remove `EnumGetters` and transfer copying behavior to `EnumAsGetters` (i.e. `as_foo()` where foo is a known copy type would return `Option<T>` instead of `Option<&T>`). After typing this out, I'm leaning towards the second option. ¹see rust-lang/rust#25893 for more details regarding "known" copy types -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #4 (comment)

[TheDan64]

That said, I'm not sure how structure/tuple variants fit into the picture since they are sort of anonymous ⁣

…

On Jul 22, 2017, 12:14 PM, at 12:14 PM, Alek Ratzloff ***@***.***> wrote: A conflict occurs: `EnumGetters` currently implements special getters for known¹ `Copy` types, which will just copy the type. I see a few options: * Keep `EnumGetters` as it is and add `EnumAsGetters` and `EnumIntoGetters` * Remove `EnumGetters` and remove copying behavior completely * Remove `EnumGetters` and transfer copying behavior to `EnumAsGetters` (i.e. `as_foo()` where foo is a known copy type would return `Option<T>` instead of `Option<&T>`). After typing this out, I'm leaning towards the second option. ¹see rust-lang/rust#25893 for more details regarding "known" copy types -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #4 (comment)

[alekratz]

Yeah, that all makes sense, the more I think about it. The user can just copy/clone how they want, and we don't need to provide any shortcuts. Works for me 👍

Also, I'm starting to think that as_* methods should not return Options. If a user is going to be calling as_foo(), they're expecting to unwrap it anyway. If they aren't unwrapping it, then they may be doing an if let Some(...) = foo or match, which adds a redundant layer since you can already use pattern matching with the enums we're wrapping. Or if they're checking against is_some or is_none, EnumIsA is implemented for all variants and has the same behavior.

Does that make sense? What do you think?

[TheDan64]

Yeah, I guess that's fair.

And as far as tuple variants, I think it's not unreasonable to return a tuple

enum Foo {
    Bar(bool, u32, f32),
}

fn into_bar(self) -> (bool, u32, f32);
fn as_bar(&self) -> &(bool, u32, f32);
// Or this one, but I think this may be equivalent to the one above
fn as_bar(&self) -> (&bool, &u32, &f32);

Struct variants seem like the most ambiguous, maybe it's best to not support enums with them altogether or to just not provide methods for those variants? At the very, least adding support for them in the future if someone thinks of a good way to do so, would be backwards compatible since you're just adding new methods.