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

send-to-self - is it a code smell? #40

Open
kamituel opened this issue Apr 24, 2017 · 2 comments
Open

send-to-self - is it a code smell? #40

kamituel opened this issue Apr 24, 2017 · 2 comments

Comments

@kamituel
Copy link
Collaborator

Hey,
I'm in the middle of migrating to systems-toolbox 0.6.x (finally! :)) and it gets me to think again about :send-to-self (formerly run-handler). It feels very weird to have to use that instead of :emit-msg.

In fact, the only reason we need it is because state component can grow very large. We can't split it, because this would mean having multiple state atoms. We don't want to do that - it makes sense (for all the reasons we discussed, and others are giving) to keep app state in a single place.

systems-toolbox kinda encourages to think there's 1-1 relationship between state maps/atoms, and components (i.e. for a single state atom, you get a single component). That doesn't have to be the case, though, does it? By having a single state atom/map, upon which multiple state components would operate, we still get the advantage of having a single source of truth, and at the same time we could split logic sensibly which should make :send-to-self simply unnecessary.

For instance, let's imagine this state map:

{:shopping-basket [...]
  :auth-token ...}

We could have one state component that would operate on :shopping-basket, and another that would operate on :auth-token. For instance:

(defn submit-purchase [{:keys [msg-payload current-state]}]
   {:emit-msg [[:backend/purchase (assoc msg-payload :auth-token (:auth-token current-state)]
               [:auth/renew-token]]})

Imagine such handler - it would submit order for processing (adding an auth token to it), and at the same time ask auth component to get a fresh auth token (in this imaginary scenario tokens can be used only once).
To write such a code, I'd need both auth and purchase components to be separate, otherwise :auth/renew-token wouldn't get delivered.

It's much cleaner than having to write:

(defn submit-purchase [{:keys [msg-payload current-state]}]
   {:emit-msg [:backend/purchase (assoc msg-payload :auth-token (:auth-token current-state)]
    :send-to-self [:auth/renew-token]})

This one is harder for a number of reasons:

  1. I have to think "is the target handler in the same component, on other component? Should I use :emit-msg or :send-to-self?"
  2. :send-to-self doesn't use put-fn, so no meta support, among other goodness.

I wonder what is your take on that?

@matthiasn
Copy link
Owner

Hey Kamil, first of all, great news about finally migrating, congrats. Clojure.spec alone is well worth it.

I don't like this send-to-self business much, either. Having multiple components use the same atom could be the way to go, not only for separation of concerns, but also for parallelization, especially when there are more reads than writes. I was just thinking about that recently, where I have many relatively expensive queries against the state, and much fewer writes. That use case would definitely benefit from having multiple query processors, without blocking the one components that would do the updates.

Clojure's atoms would provide all the coordination needed here, however our handlers would need to return a function that can be applied in a swap!, and retried if two tried to alter the atom at the same time. We couldn't use :current-state and :new-state any more then, just :current-state for read-only operations, or else we'd have to implement the retry semantics that the atom already provides. Or alternatively just interact with the atom inside the handler directly.

Do you think that would make sense?

@kamituel
Copy link
Collaborator Author

kamituel commented May 2, 2017

Hm, that's true, and that would be going back one step almost to using cmp-state directly. However, the notion of using {:new-state ... :emit-msg ....} might get some refactor anyway.

I'm still in the middle of migration (other things got priority over that for a few days), and I tried to move from cmp-state/put-fn to returning a map from handlers. It's all good for the most part, but some handlers are just inherently complex. For instance, I might want to delegate some of the handler logic to the helper function, and then do some final touches in the handler itself and return.

Something like:

(defn helper
  [...]
  ;; Helper wants to emit some messages, or alter state, as well.
  {:emit-msg ....
    :new-state ....})

(defn handler
  [{:keys [current-state]}]
  (let [helper-ret (helper ...)]
     ;; Handler wants to do all the things helper did, and also do some more
     ;; (i.e. emit more messages or alter state - but alter state based on the
     ;; state that helper returned, not the "current-state".
     (__merge__ helper-ret {:emit-msg ....})))

I hope this is a clear example. It's manageable of course, but leads to an either ugly, or verbose code.

One idea I was thinking about would be to use something like this instead:

(defn handler
   [...]
   [{:emit-msg ...}
     {:alter-state assoc :page 3}
     {:emit-msg ...}
     {:alter-state dissoc :data}])

So - a sequence of actions that handlers wants to do (which will be carried out by systems-toolbox). With this format, building on top of helper's return value is a simple matter of using conj or something like that.

This approach would also kinda solve the issue you described above too, right?

I don't like the idea of referring to assoc and dissoc directly in the example above (this would lead to ugly log messages, for instance), but that's something to get around surely somehow.

What do you think?

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

2 participants