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

Strategy #146

Closed fade2black closed 3 years ago

fade2black commented 3 years ago

Please review.

Closes #120

pickfire commented 3 years ago

There is multiple way to do this, one is multiple files/modules, one is using feature flags, one is multiple crates. Maybe we should mention these methods on how to provide different strategies?

By the way, are you running gofmt rather than rustfmt here? It would be nice to just rust the rust code through rustfmt and add it there. It would also be good to have some use cases/test cases for the examples, could be done using assert_eq!.

simonsan commented 3 years ago

@fade2black I did a bit of layouting before to actually track the changes better, don't be surprised when you pull the changes from your repo. ;-) Cheers.

fade2black commented 3 years ago

There is multiple way to do this, one is multiple files/modules, one is using feature flags, one is multiple crates. Maybe we should mention these methods on how to provide different strategies?

@pickfire I agree. Actually, I mentioned it in the Discussion section in a single sentence: "Typically in Rust, each strategy should be implemented in a separate module file.". I'll add more on that.

simonsan commented 3 years ago

👍🏽 Nice work ;-)

fade2black commented 3 years ago

@simonsan While I was looking through patterns on this repo, I noticed that people does not mention or refer to SOLID principles at all. When designing application we usually think in terms of design principles, e.g, DRY, SOLID, etc, and only then we choose specific patterns to achieve our goals. I think it would be, in particular, useful and informative to create a section for design principles too. I mean, sometimes Strategy, Command, etc,...patterns do not mean much. But if I know, for example, that a specific pattern allows me to to stay DRY, then I consider it. Personally for me, software design is built upon principles, and those principles are achieved using concrete patterns. Just my opinion :-)

simonsan commented 3 years ago

@fade2black Opened an issue to discuss that ;-)

MarcoIeni commented 3 years ago

I am rewriting this here because the conversation was resolved. fade2black, what do you think about removing all the Result in the example by substituting write! with something like format! and push_str?

fade2black commented 3 years ago

@MarcoIeni Agree. I'm removing Results.

fade2black commented 3 years ago

@MarcoIeni Did you have in mind this?

data.iter().map(|(key, val)| format!("{} {}\n", key, val)).collect();
fade2black commented 3 years ago

@MarcoIeni I use return s instead of simply typing s. s alone looks lost 😀

MarcoIeni commented 3 years ago

@MarcoIeni Did you have in mind this?

data.iter().map(|(key, val)| format!("{} {}\n", key, val)).collect();

Exactly! :heart:

@MarcoIeni I use return s instead of simply typing s. s alone looks lost

So let's remove both! :stuck_out_tongue_winking_eye: Suggestion incoming..

MarcoIeni commented 3 years ago

I approved, but I don't merge this because maybe simonsan and pickfire want to review this as well :)

Anyway thank you so much for your contribution!!

fade2black commented 3 years ago

@MarcoIeni Thank you too for your suggestions and collaboration!!!

simonsan commented 3 years ago

r? @pickfire

MarcoIeni commented 3 years ago

I think it's time to merge this! If pickfire or anyone else want to propose changes they will be able to do a PR whenever they want! :) I proposed some changes that should fix the CI. Once the CI passes I would like to merge this.

simonsan commented 3 years ago

I think it's time to merge this! If pickfire or anyone else want to propose changes they will be able to do a PR whenever they want! :) I proposed some changes that should fix the CI. Once the CI passes I would like to merge this.

Nothing for a # See also section? :(

MarcoIeni commented 3 years ago

Maybe fade2black have some useful links to share!

fade2black commented 3 years ago

@MarcoIeni @simonsan I have added See also section. But I had to remove this link Policy Based Design from the section because the CI strangely complaints it is a dead link.

fade2black commented 3 years ago

@MarcoIeni Passed.

fade2black commented 3 years ago

@simonsan @MarcoIeni Serde is mentioned as an example.

simonsan commented 3 years ago

Opened upstream issue: https://github.com/gaurav-nelson/github-action-markdown-link-check/issues/96

simonsan commented 3 years ago

Really nice, thank you! <3