rust-unofficial / patterns

A catalogue of Rust design patterns, anti-patterns and idioms
https://rust-unofficial.github.io/patterns/
Mozilla Public License 2.0
7.95k stars 362 forks source link

Add "Use Self in impl" idiom #145

Open MarcoIeni opened 3 years ago

MarcoIeni commented 3 years ago

Idea from #61 Should we add an idiom that suggest to use Self inside impl? See the PR above for an example.

pickfire commented 3 years ago

There is a reason this is not added last time and left hanging, it is due to no consensus on whether using Self is better, personally I would only try to use Self when the type is very complicated (so in this case using Self is easier to read) but I use the original type in almost any other cases to make it easier to read.

I don't think it is a good idea to write it in the idiom part since it makes not doing so looks bad.

simonsan commented 3 years ago

I think even stating what you just said, comparing it and giving information on that matter is already valuable enough to have it in the repository. To give people the knowledge of other people at hand they can think about adopting. These are no rules.

I see this repository more as a collection of collaborative collective knowledge than "a statement of what people need to do and what not". So imho it's not really about if Self or FullType is better but in which situations is each of it good to use and where not.

MarcoIeni commented 3 years ago

Ok, so basically our decision is to write this including also the content of the comment of pickfire, right? If you want to write it leave a comment!

pickfire commented 3 years ago

I wonder if we should write this? Or maybe we can just keep note of this while writing other examples?

MarcoIeni commented 3 years ago

I wonder if we should write this? Or maybe we can just keep note of this while writing other examples?

IMHO other examples should link to this idiom. Otherwise you duplicate the explanation of this concept all over the place.

pickfire commented 2 years ago

One more issue besides missing impl context is that it won't work well with rustdoc search. If you search for the original type using the search bar for docs, it won't show up in return types tab. Let's add one more item:

Disadvantages: (search) discoverability

Example Client can be created with connect but probably because it uses Self which prevents the search from finding it https://docs.rs/grammers-client/0.3.0/grammers_client/client/client/struct.Client.html?search=client#method.connect

simonsan commented 2 years ago

One more issue besides missing impl context is that it won't work well with rustdoc search. If you search for the original type using the search bar for docs, it won't show up in return types tab. Let's add one more item:

Disadvantages: (search) discoverability

Example Client can be created with connect but probably because it uses Self which prevents the search from finding it https://docs.rs/grammers-client/0.3.0/grammers_client/client/client/struct.Client.html?search=client#method.connect

If that would be the case that's more an issue with rustdoc imho and might be worth to be opened over there. Anomalies in rustdoc shouldn't be an argument for or against something we discuss here imo.