Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

converge.api/pop-patches! vs. IConvergent/-pop-patches! behavior surprising #19

Open
alpeware opened this issue Nov 15, 2020 · 0 comments

Comments

@alpeware
Copy link

Hey Bobby,

I'm trying to use refs in my current project, so filing some issues as I'm learning - I hope you don't mind.

I was surprised that the api implementation updates the queue and returns a patch vs. the protocol implementation returning the rest of the queue.

(defn pop-patches!
  [cr]
  (let [p (peek-patches cr)]
    (ref/-pop-patches! cr)
    p))

vs.

(-pop-patches! [this] (set! patches (pop patches)))

It looks like the readme has this wrong as well -

(convergent/pop-patches! c)
;=> removes the first locally-made change from the queue of patches, returning the new queue

A suggestion might be to update the api fn to pop-patch! instead or borrow from core.async and use poll-patch!.

Taking the opportunity to simplify another readme example a bit as well, we would get something like this -

(while true
 (if-let [patch (convergent/peek-patch! c)]
  (do-something-with patch) ;; e.g. send to remote actors
  (Thread/sleep 1000)))

or

(while true
 (if-let [patch (convergent/poll-patch! c)]
  (do-something-with patch) ;; e.g. send to remote actors
  (Thread/sleep 1000)))

Interested in your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant