Criticism for Simple Optic Library

I recently started (seriously) learning Clojure and as a beginner project I wrote a small library which implements basic optics. Here is a link to the source code. I’d appreciate all kinds of criticism but I am mostly interested in

  • bad code style (bad formatting, wheels I reinvented, unidiomatic code, etc)
  • missed opportunities for using macros (I haven’t defined any, maybe I should have at some places?)
  • missed opportunities for abstraction (I used a few defrecords but no deftypes, multimethods, interfaces, etc)
  • comparison to similar libraries (I am not too familiar with the ecosystem).

Thanks in advance!

1 Like

I will try to find some time to look at it and share some constructive ideas! I just wanted to chime in quickly before bed to say that macros are not something one looks for opportunities to use, they are a serious tool one brings out when one has exhausted the opportunities of finding any other reasonable solution. :grinning: I went through a phase of looking for reasons to use them, and got bit by the things they make confusing or awkward (they aren’t first class, like functions, so you can’t compose them or pass them around, for example). Now I use them only when they truly are necessary.

2 Likes

I’ve time to take a little look now, so here are some early thoughts. I don’t know concrete optics, and haven’t studied the data structures chosen yet, so this is superficial style review for now. Looking at base.clj, do you want typo reports, like the fact that “single” is misspelled in the doc-string in line 7?

The first thing that jumped out at me is that preview returning :nothing if it fails is non-idiomatic, it’s much more common in such cases to simply return nil, which is also falsey. And that would simplify the code, as well, since nil is generally safe to do things with because of nil-punning (my snippets for illustration here are compressed and omit the doc strings, you would stick with the way you actually wrote them):

(defn preview [optic whole]
  (let [lst ((get-capability :to-list optic) whole)]
    (first lst)))

That becomes so simple that you don’t even need the let-binding. But even if there is a domain-specific reason that returning :nothing is more useful than nil, a more idiomatic way of writing your last expression would be:

(or (first lst) :nothing)

Since you are looking for opportunities for abstraction, there are several functions that look more or less the same, view, to-list, and review for example, could all be one function, if we could come up for a good name for it (here is my lack of domain knowledge interfering). But say we called it find, you would pass it the optic and either a whole or part, which we could perhaps call part since a whole is a part, right? :smiley:

(defn find [optic part k]
  ((get-capability k optic) part))

Then the three functions I mentioned would just be calling this with values for k of :view, :to-list, and :review, respectively.

You might even be able to build this up with additional optional arguments to encompass some of the other functions which apply transformations to the result of calling get-capability.

I would definitely caution against writing or using a function like rev-comp. It’s non-idiomatic and very limited compared to comp, taking only two functions and building a function that takes only one argument. comp can take an arbitrary number of functions which each take an arbitrary number of arguments. Experienced Clojure developers are used to comp and the order it applies functions (which is also consistent with the mathematical notation for composition of functions), so it would benefit you to get comfortable with that rather than introducing an incompatible and limited replacement.

But I should delve deeper into some of the functions you are calling here before speculating more at the top level, let’s see when I have some more time. But even before then, please let me know if this is the kind of thoughts you are looking for?

Oh, I did have one more thought. The existence of apply-binary-nonnil is a little worrisome, it would be more Clojure-like to make sure that all the binary operators themselves handled nil gracefully—that is the nil-punning that I was talking about in my last response; most Clojure core libraries treat a nil input as the base case of whatever they were expecting, so if they were expecting a map, set, vector, or list, it is treated like an empty map, set, vector, or list, etc. If all the binary operators were coded that way (perhaps just using the same logic you did in apply-binary-nonnil), you would have no need for that function, and the binary operators would be as pleasant to use as the core library functions.

Thanks for the feedback!

I don’t know concrete optics

There is nothing called “concrete optics”. It is just the name I chose for the library since I am not using any fancy stuff like profunctors, existential types of van Laarhoven embeddings.

do you want typo reports

If they lead to confusion, yes. You can skip the trivial typos, though.

preview returning :nothing

I understand what you mean and that was what I originally planned to do. However, if you use nil then (predicate-prism nil?) is broken. Currently (predicate-prism nothing?) is broken. In any case something will be broken since this is Clojure. I think (predicate-prism nil?) is much more likely to be used since :nothing is specific to this library. Maybe I can rename :nothing to something like :no-match-in-prism to emphasize that it is something specific to this library. In any case, good call! I will clarify this in the docs.

a more idiomatic way of writing your last expression would be: (or (first lst) :nothing)

Ah! This is very good. I will refactor the code.

view, to-list, and review for example, could all be one function

Hmm. I think there is a misunderstanding here, or the fact that the untyped nature of Clojure is making things obscure. There is a very good reason they are distinct. They may look similar but they compose very differently. For instance review and view are in opposite directions, so you cannot compose them. However two views can compose.

I would definitely caution against writing or using a function like rev-comp.

It is used in a single place and it is not exported. So I did not pay attention to it. But I understand your point. What do you think about something like this?

(defn rev-comp [functions]
  (apply comp (reverse functions))

But even before then, please let me know if this is the kind of thoughts you are looking for?

Yes this is precisely what I am looking for. But I am also interested in your thoughts about the documentation. If you have time, you can have a look at the showcase.clj module under tests.

Oh, apply-binary-nonnil comment wasn’t here when I started writing.

so if they were expecting a map, set, vector, or list, it is treated like an empty map, set, vector, or list, etc

They are all different kinds of functions. So there is no base case here.

If all the binary operators were coded that way

They are not. For instance I would want the composition of nil with any function to be nil but we have

user=> (nil? (comp nil identity))
false

So comp does not handle nil “gracefully” --though I am too much of novice to understand what graceful means here as I can imagine several defensible behaviors for (comp nil identity).

perhaps just using the same logic you did in apply-binary-nonnil

So should I duplicate the logic in binary operations? Or do you want me to partially appply apply-binary-nonnil to comp, rev-comp and vector-fish?

Some general observations:

If you turn on reflection warnings via (set! *warn-on-reflection* true)
you will see all the unhinted method invocations in structures light up with warnings on load. That will cost you substantial performance (sometimes we don’t care though, it’s context dependent - like if we are only hitting that path 1x or very rarely it’s no big deal [with maybe caveats for stuff like graal native compilation]).

I think hints on the top-level defs helps:

(def ^Semigroup additive-semigroup
  (Semigroup. +))

(def ^Semigroup first-semigroup
  (Semigroup. (fn [x _y] x)))

(def ^Semigroup vector-semigroup
  (Semigroup. (fn [x y] (into [] (concat x y)))))

;; A monoid is a semigroup with a unit element.
(defrecord Monoid [binary-op unit])

(def ^Monoid additive-monoid
  (Monoid. (.binary-op additive-semigroup) 0))

(def ^Monoid vector-monoid
  (Monoid. (.binary-op vector-semigroup) []))

Another option is, instead field accesses you could introduce a protocol like

(defprotocol IBinary
   (bin-op [this]))

(defrecord Semigroup [binary-op]
   IBinary
   (bin-op [this] binary-op))

(def additive-semigroup
  (Semigroup. +))

(def  first-semigroup
  (Semigroup. (fn [x _y] x)))

(def vector-semigroup
  (Semigroup. (fn [x y] (into [] (concat x y)))))

;; A monoid is a semigroup with a unit element.
(defrecord Monoid [binary-op unit]
   IBinary
   (bin-op [this] binary-op))

(def additive-monoid
  (Monoid. (bin-op additive-semigroup) 0))

(def ^Monoid vector-monoid
  (Monoid. (bin-op vector-semigroup) []))

Then all the hinting/typing is lifted into the protocol and since the protocol definitions are inlined they’re as efficient as the field accesses from earlier.

You could also just leverage keyword access (at some minor perf penalty, but substantially faster than reflective access) to get at the entries of the record:

(def ^Monoid vector-monoid
  (Monoid. (:binary-op vector-semigroup) []))

You can do similar stuff for fmap and the like (plenty of clojure FP libs like fluokitten already provide these implementations).

It’s typically faster to use the function invocation for keyword lookup on maps (same with indices in vectors, or entries in sets) and remain idiomatic:

({:a 1} :a);;faster path
(:a {:a 1});;slower, around 2x but on nano-scale, still way faster than reflection

Except records don’t implement clojure.lang.IFn out of the box - you have to define the keyword lookup via function yourself so you can’t use this idiom without some mods or a little macro wrapper around defrecord.

Could be viable to ditch records altogether and avoid the type proliferation (consolidate on generic maps), but maybe you have some unspoken tradeoffs with that route. If you don’t need inline protocol definitions or direct field access for static (possibly primitive) fields (typically for efficiency), it’s unclear what influences the choice of records (aside from, possibly, writing pseudo Haskell in Clojure and looking for analogues, or getting additional readable map types for e.g. syntax tree structures).

1 Like

Another option is, instead field accesses you could introduce a protocol like

This seems like a much better approach. Falls under the ‘missed opportunities for abstraction’ category.

aside from, possibly, writing pseudo Haskell in Clojure

It is the case. What I wanted to do was to port something I understand well from Haskell to Clojure in a naive way and ask the community about how to translate it into idiomatic Clojure --assuming it is possible and advisable to do so.

plenty of clojure FP libs like fluokitten already provide these implementations

This is what I meant by ‘the wheels I reinvented’ in the post. I will definitely have a look at this library and probably use it --I see that applicatives are already there.

This was very helpful. Thank you very much!

There is a difference with Clojure and Haskell. I’m not very good at Haskell and really admire the people that are. Maybe I’m simplifying it a bit but from my point of view the reason why Category theory is so useful in Haskell is because you use it to leverage the compiler. When you move into dynamic languages, you don’t get the leverage but you don’t get the straitjacket either. Therefore all those concepts that work really well in Haskell now does not offer the same bang for buck - Haskell style Clojure then becomes verbose and archaic to those that don’t know Haskell or are not well versed in Category Theory.

I know I have monads in my code. It’s such a useful pattern that naturally appears especially when attempting to cleanup/refactor code. I honestly can’t tell you where I’ve written them though and from a practical standpoint it doesn’t matter as long as the code got the job done. Tagging everything in your Clojure code Monad or Semigroup won’t make it go any faster.

Your code is probably organised a little bit too well, as in you can merge maybe 4 or 5 files into one. It’s a lens library so I don’t think it should be as complicated as it’s currently written. I’ve seen a few done in about 200 loc with really good examples too. I don’t actually know what data structure you are targeting (atoms?), reading the documentation, there are no examples of the high level use. It’d be great if you can add some examples.

Thank you very much for the feedback!

Maybe I’m simplifying it a bit but from my point of view the reason why Category theory is so useful in Haskell is because you use it to leverage the compiler. When you move into dynamic languages, you don’t get the leverage but you don’t get the straitjacket either.

I agree. Lenses were discovered by database people, the first examples were table views. Category theoretic interpretation came later. I am not ‘using’ any category theory. Actually, my implementation is very un-category theoretic. A direct translation of the implementation in Clojure to Haskell would be very unidiomatic and utterly unusable. Where did you see category theoretic influence in the code?

Tagging everything in your Clojure code Monad or Semigroup won’t make it go any faster.

I think I am missing the point here. I did not tag anything.

Your code is probably organised a little bit too well, as in you can merge maybe 4 or 5 files into one.

Yes, I also felt that way after a certain point. Probably I will have one module for structures and one module for axioms.

It’s a lens library so I don’t think it should be as complicated as it’s currently written.

It is an optics library, not just a lens library. It has isos, lenses, prisms and traversals. Can you point me to the examples you mentioned? I found a few lens libraries but nothing covering all four main optics --admittedly didn’t search too hard.

there are no examples of the high level use

there is a showcase with several examples: Showcase

Also there is a high level explanation: Introduction to concrete-optics

Did you miss them or should I add more?

i did miss the showcase. that helped to explain things alot. i don’t know the exact terminology but i can see how powerful these abstractions can be.

it’d still be great to showcase an actual problem that you’re solving. Those trivial examples don’t immediately wet people’s appetite for delving more deeply into it.

it reminds me of spectre and some of the tree walking techniques I used implementing GitHub - zcaudate-me/jai: moved -> 'lucid.query'

You also mentioned its use in databases, do you know which part of the query it is used for?

I did see a couple of ‘monoid’ references in the code and my mind immediately went to Haskell weeny. Apologies and my fault for being to premature to judge.


axioms refer to core so it is at a higher level right? it’s confusing when you have code in subfolders that are at different levels in terms of the code hierarchy

I’m not that well versed in Category theory. I had only seen what previous people came out with when they mentioned optics. A quick google search shows:

From the examples it just looked like a two way version of get-in and assoc-in to me so I just couldn’t really get excited about it. What would be the biggest difference between optics and zippers, which for me seem far more useful and generic.

Those trivial examples don’t immediately wet people’s appetite for delving more deeply into it.

At this stage I am not inviting people to delve in, though I think the examples I provided are not worse than, say, the ones provided by the specter docs. But if you want it spelled-out you can use what Specter docs say: Specter concrete-optics rejects Clojure’s restrictive approach to immutable data structure manipulation…

As a reminder, I am doing this to learn about Clojure. I wouldn’t offer concrete-optics as a serious library before I add benchmarks, gather more feedback and learn about the best-practices, conventions, limitations and taboos of Clojure.

You also mentioned its use in databases, do you know which part of the query it is used for?

I mentioned that example to emphasize that optics in isolation are not inherently category theoretic. There is a pattern in database theory which fits the lens specification. It is not about this library. Here is paper about it. It gets more technical as it progresses but the introduction and the first paragraphs of section 2 should give you an idea.

Apologies and my fault for being to premature to judge.

No need to apologize. Choice of terminology does have that affect. People seem to be perfectly fine with continuation passing style unless you mention the Yoneda embedding.

axioms refer to core so it is at a higher level right?

Axioms are meant to be used in tests. Both in the tests of concrete-optics itself, and in the tests of any project importing concrete-optics and making its own optics using one of the make-<optic> functions. For instance, in the showcase I defined an isomorphism between Celsius and Fahrenheit using mk-iso, and also wrote property tests for it using the axioms provided by the library.

Oh I had seen these examples:

  • Adashot: only lenses
  • spectacles: not even a general lens, just usual getter/setter/modifiers with extra bells and whistles
  • active-closure: only lenses
  • lentes: only lenses.

The only I managed to find which comes close is coarse. Your criticism about using category theory directly would probably apply to this implementation. Also it does not have isomorphisms and prisms.

What would be the biggest difference between optics and zippers, which for me seem far more useful and generic.

I had a quick look at the zippers in Clojure. They seem to be the classical zippers: they allow you to talk about relative positions of points in nested data --though please let me know if I missed the point. So, for instance, I don’t know how to replicate the validation examples in the showcase using zippers. In those examples I validate and modify a nested structure simultaneously and if there are errors in the validation step I can pick the first error and stop or collect all errors or count the errors or determine the maximum error severity if a ranking is provided or … And to achieve that I do not touch the traversal I used on the structure, I only change the applicative I use while traversing. So the point here is modularity and code re-use. I can use the same applicatives with other tarversals and get the same result: say with vectors without nesting, trees made of vectors, trees made of maps, fixed depth associative structures etc. So traversals decouple ‘walking strategy on the whole’ and ‘reassambling strategy on the parts’. The bonus is that it fits into a general framework which contains lenses, prisms and isos. So for instance you can validate virtual fields --which, I believe, zippers cannot do. And I chose Validation as a running examples but this method is by no means limited by validation.

Again, thank you very much for spending time on this and providing feedback. The discussion is not going in the direction I was expecting but I am certainly enjoying it.

Really cool stuff. Thanks for clarifying. My prev contact with optics stemmed from those libs so I had assumed that optics = lenses.

My only feedback code wise would be to move all the structure.clj into one file and all the axiom.clj into one file. Really good stuff.

Can you explain more about how lenses work over a relation model? Can you do lens recursion and stuff? What can I do with something like graphql data?

What are virtual fields? like computed fields?

It’s possible to use a zipper to do something like error collection/validation if one goes the impure route and caches a bit of state. Zippers are really tiresome to code because one needs to be super explicit but they are really good for code manipulation. Am I right in understanding is that optics add a little bit more information to the traversing structure so that it can do more funky stuff?

I thought I was missing out until I realised all those were mainly used as workarounds to allow FP programmers access to an imperative programming syntax. Can’t wait for someone to tackle assembly.

… so I had assumed that optics = lenses.

Oh there is an entire zoo of lenses: Fold, Setter, Fold1, AffineFold, Getter, Traversal1, AffineTraversal, PrimGetter, Getter, Review, Grate, Achromatic… There is no complete list. One can still discover new optics. I opted for a closed design though in concrete-optics to keep things simple.

Can you explain more about how lenses work over a relation model?

Sorry but I will again reply with a paper

Can you do lens recursion and stuff?

I am not sure what you mean by lens recursion.

What can I do with something like graphql data?

I don’t know, I don’t have an example. Maybe you can see instances of lenses and lens composition in a graphql library if you squint hard enough? It is natural object like monoids.

What are virtual fields? like computed fields?

From the showcase: first example, second example.

Am I right in understanding is that optics add a little bit more information to the traversing structure so that it can do more funky stuff?

Well, that is the iterator pattern. Traversals allow you to use that pattern. I could have written a smaller library on the iterator pattern only. The fact that they are also optics --that is, you can compose them with other optics-- is what makes them more useful in my opinion.

Did you just google and send me the first result? That was the exact same paper I looked at before asking the question.

I’m sure that is the case but I find that the best ideas promote themselves.

edn itself and any operation transforming edn to edn satisfies this property. I don’t need to squint. I just think it’s stupid to talk about these thing beyond the field of mathematics.

Did you just google and send me the first result? That was the exact same paper I looked at before asking the question.

No.

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.