search

rust-unofficial / patterns

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

New Section: Lenses and Prisms #326

Closed jhwgh1968 closed 1 year ago

jhwgh1968 commented 1 year ago

As the intro says, this is a pure functional concept that Rust doesn't often use directly, but can be used to help understand some very "functional" looking APIs, such as Serde.

It is a rather difficult concept, even for me, so the wording might not be the best. I'm open to any feedback and improvements.

jhwgh1968 commented 1 year ago

Hmm, it looks like I didn't finish some of tho code examples...

I'll be fixing the code in the next couple hours for hours. Hopefully that should improve clarity, too.

At the very least, what about the topic and the Serde idea? That's what motivated this.

simonsan commented 1 year ago

The topic is a good idea! Please be aware to put each new sentence on a new line for tracking diffs. Will be on holiday the next week and make a proper review afterwards, I guess you will add some stuff during that time, so it makes sense to wait with the review till then?

jhwgh1968 commented 1 year ago

Sounds like a plan!

jhwgh1968 commented 1 year ago

Thanks for your comments @neithernut. They are all good points.

I will re-write this a little to address them.

jhwgh1968 commented 1 year ago

I have done a fairly major re-write and reflowed and made the content more specific, tightening down my use of words as per commenst by @neithernut.

I will wait to fix the minor markdown issues until I can get a big picture review.

simonsan commented 1 year ago

Shall we finalize this? @jhwgh1968 (as in reviewing and merging or is there still something from your side todo?)

jhwgh1968 commented 1 year ago

Now that I've fixed the last set of linter issues, it's ready as far as I'm concerned!

neithernut commented 1 year ago

The school-of-haskell link is still somehow misparsed as a local reference.

jhwgh1968 commented 1 year ago

It looks like that did indeed fix it. Thanks @simonsan

simonsan commented 1 year ago

It looks like that did indeed fix it. Thanks @simonsan

Wasn't me ;-)

MarcoIeni commented 1 year ago

Looks good to me! Thanks for your contribution and all the other reviewers. :) This PR has seen many changes already, so I suggest merging it and doing additional refinements in other PRs, if needed. I only left some minor comments. What do you all think?

simonsan commented 1 year ago

Looks good to me! Thanks for your contribution and all the other reviewers. :) This PR has seen many changes already, so I suggest merging it and doing additional refinements in other PRs, if needed. I only left some minor comments. What do you all think?

Want to read through it tomorrow evening again, and also review. Haven't had time as much this week.

jhwgh1968 commented 1 year ago

I have applied the minor changes @MarcoIeni requested, and it has now passed CI.

simonsan commented 1 year ago

Great read, thank you for this awesome contribution! <3