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
7.84k stars 352 forks source link

Iteration on Functional optics #364

Closed jhwgh1968 closed 10 months ago

jhwgh1968 commented 1 year ago

An early draft to re-write the lenses section as functional optics.

Opened for early comments from @simonsan and @thomasmarsh

Not formatted or link validated yet, I'm looking for a broad content review an accuracy check.

Currently has other CI stuff mixed in to help me maintain my own fork, do not merge.

Will eventually deal with #329

simonsan commented 1 year ago

Sorry, for taking this long. I will be able to look at it in the upcoming days. (:

jhwgh1968 commented 11 months ago

@simonsan or @thomasmarsh, I imagine this slipped through the cracks.

Any comments?

simonsan commented 11 months ago

@simonsan or @thomasmarsh, I imagine this slipped through the cracks.

Any comments?

Oh yes, indeed. It slipped through here! It's bedtime here now, but will definitely look into it tomorrow or Tuesday! Sorry :(

simonsan commented 11 months ago

@jhwgh1968 could you please remove all the changes to the CI and make the changes against the lenses.md (so renaming optics.md back to lenses.md) first so we can review against it for the overall changes and then before merging we can rename it?

jhwgh1968 commented 11 months ago

Oh yes, indeed. It slipped through here!

No problem. You'll notice I forgot to remind for months :wink:

could you please remove all the changes to the CI and make the changes against the lenses.md (so renaming optics.md back to lenses.md) first so we can review against it for the overall changes

Being a complete re-write without even formatting fixed yet (because I expect to significantly rework it), I didn't expect it to be a problem.

Done and done.

simonsan commented 11 months ago

could you please remove all the changes to the CI and make the changes against the lenses.md (so renaming optics.md back to lenses.md) first so we can review against it for the overall changes

Being a complete re-write without even formatting fixed yet (because I expect to significantly rework it), I didn't expect it to be a problem.

Done and done.

Ahhhh! You rewrote it completely! Got it, thought you were "just" changing many things. Indeed the rename doesn't make much sense then. I was fooled by reading the introduction sentence and thinking both will be nearly the same with many additions. 😅 Anyway, we can rename it back in the very end now, sorry for the confusion.

jhwgh1968 commented 11 months ago

Thanks for catching that stuff, @simonsan. I fixed it all, and added more of an overview paragraph.

jhwgh1968 commented 10 months ago

I am hoping you can get to this soon, @simonsan. I have roughly a month before I will disappear for quite a while, and am hoping to get this in.

If the new beginning looks good, and the material is understandable, I will take it out of draft.

simonsan commented 10 months ago

I am hoping you can get to this soon, @simonsan. I have roughly a month before I will disappear for quite a while, and am hoping to get this in.

If the new beginning looks good, and the material is understandable, I will take it out of draft.

Yes, I will give it another read the next days! 🤞🏽

simonsan commented 10 months ago

@thomasmarsh Would be happy, if you can give it a review as well. :)

jhwgh1968 commented 10 months ago

Comments fixed, @simonsan.

While I am hesitant to speak for @thomasmarsh, I would note that their account has been inactive for over 6 months. (I'd check their Birdsite account, but I don't have one there myself, so I can't see it with recent management decisions.)

simonsan commented 10 months ago

@jhwgh1968 Not sure, if you heard about it, there is another (de-)serialization framework by udoprog: https://github.com/udoprog/musli

Where Müsli differs in design philosophy is twofold:

  • We make use of GATs to provide tighter abstractions, which should be easier for Rust to optimize.
  • We make less use of the Visitor pattern in certain instances where it's deemed unnecessary, such as when decoding collections. The result is usually cleaner decode implementations.
  • Another major aspect where Müsli differs is in the concept of modes (note the M parameter above). Since this is a parameter of the Encode and Decode traits it allows for the same data model to be serialized in many different ways. This is a larger topic and is covered further down.

I find this quite interesting and would be interested in your opinion regarding that, if it makes sense to name as another approach or something?

jhwgh1968 commented 10 months ago

Link added, @simonsan. How does it look now?

simonsan commented 10 months ago

Looks good to me, let's give it till Sunday evening this week, and if no comments on changes show up, we'll merge, ok?

Tagging for visibility: @MarcoIeni @pickfire @thomasmarsh

simonsan commented 10 months ago

@jhwgh1968 Thanks a lot (also for the reminder ;))! 🚀