State class is more general concept

[hleumas]

Let's become familiar with Clojure atom:
http://clojure.org/atoms

It is very similar to este State.
atom

(def a (atom value)) // create new atom
(deref a) // get the value
(swap! a f arg1 arg2 ... argN) // set the new value to (f value arg1 arg2 ... argN)

State

let a = new State(value); // create new state
a.get() // get the value
a.set(newValue) // set the new value

In both Clojure and Este, atom/State can be watched and should contain immutable values. In both of them, they can be watched for changes.

However, I think it makes sense to separate the concepts of atom and State. First one should be thought about as general data structure, second is container for our application state. The latter needs some additional functionality beyond the atom (like cursors) and these should be separated.

Let's create general purpose class Atom with a subset of State methods: set, get, and constructor. That's it. No load, toConsole, save, nor cursor.

[PetrSnobelt]

Hi, I added undo/redro to State in my fork
https://github.com/PetrSnobelt/este/blob/master/src/lib/state.js

Do you think it should be added to State or to something like StateWithHistory (which wrap State or there is a better option?

[grabbou]

I am not entirely sure if this kind of encapsulation is needed here. I am afraid another general data structure on top of Immutable might be unnecessary complication. Although I agree State is similar to Atom, I'd rather say Atom is more similar to top-level Immutable objects being held in State. State is a container for application state, so methods like set and get are just nice wrappers that proxy calls to underlaying Immutable objects (and makes working with immutable structures a dream - kudos to @steida for such a solution).

@PetrSnobelt - as per your example, it actually gave me another idea - when having a single app state - it might be unnecessary to implement redo/undo capabilities to all the items (this may cause some bugs or unwanted actions). Although redo action is nice to clear the form errors, I am actually thinking what will happen if we:

• subclass State and create RedoState
• put RedoState in a State
  so you can have nested States with different features... (e.g. todos cursor might be held in RedoState which is held in general purpose State). This will allow you to subscribe to more generic onChange events e.g. notify me only when undo todo happens

WDYT?

[hleumas]

@grabbou check the pull request hleumas@f3a0b44

You can simply dump the State class and use Atom instead. And Atom is already general enough. It does not contain state specific methods and assumptions. Cursors were factored to separate Cursor class and also made more general (now you can create even cursor from cursor). toConsole method was dumped and replaced with proper toString, toSource and inspect methods.

[grabbou]

It makes more sense now! Thanks for the example! Atom & Cursor make a lot of sense, unlike Atom & state :) I believe we should refactor this out from Este and publish under separate org repo so others can profit as well!

[hleumas]

@grabbou makes sense. Will create separate repo with more immutable helpers.

[grabbou]

Nice, you can give me link later on so maybe i can help.

[hleumas]

@grabbou Here it is. All suggestions welcome! https://github.com/vacuumlabs/immutable-utils

[PetrSnobelt]

Hi @hleumas, are there benefits of using custom cursor implementation over Immutable contrib cursor? https://github.com/facebook/immutable-js/tree/master/contrib/cursor

[hleumas]

@PetrSnobelt yes, Immutable contrib cursor mixes concepts of Atom, Cursor and Collection together. I don't think this is good. You should be explicit whether you work with immutable data collection or reference to that collection (e.g. Atom, or Cursor). Also, it is strange that you need to supply onChange method to Cursor constructor, that the other cursors are not update automatically, etc...

[steida]

@hleumas Can you summarize why we should add more abstraction to current solution? Every abstraction has to have pretty good reason to entre. Also https://twitter.com/swannodette/status/603970083870920704 "Next om will have no cursors."

[hleumas]

@steida It is nice to work with immutable data structures, but you need to have something mutable in your code, because our apps have state. Thats the reason you created State class in the first place, right?

That's the reason why in Clojure there are atoms, refs and agents.

If you take a look at the current State class, you can realize it is just polluted Atom. You can immediately dump State class and replace it for much simpler (and much general) Atom.

Este currently uses cursors. If you want to dump them entirely, I will be happy to see the solution. I am really curious how om will drop cursors.

I just created Cursor object that is really simple (2 lines of code if we strip whitespace, toString, class declaration and other boilerplate) and serves the same purpose as your cursor method better. It behaves as special type of reference, it has the same methods as Atom.

Look, whole code is really simple. It's just two classes, each of them having two one-line long methods and some declaration boilerplate. It is actually less code than State class.

[steida]

Sorry, but I don't really see an advantage of this abstraction.

[hleumas]

@steida let's say I renamed Atom and called it State. Would it be more acceptable?

[steida]

I like design principle behind PR, but overal it's too noisy for Este. We really don't want such facades like https://github.com/vacuumlabs/immutable-utils/blob/master/src/functions.js Would you force developer to debug/think about it? What if immutable api will change? This is no go.

Also, there is no reason to change current api (like deref), for sake of familiarity with David's OM.

Current global app state management is intentionally placed in one class. It must be easy to reason about, and spliting state and cursor is not worth it.

[grabbou]

+1, basically what I said before, I just don't think this kind of abstraction is needed here, we aim to avoid over-abstracted patterns and frameworks so let's stick with something that's easy to debug yet to tweak in developers intended way.

[hleumas]

@steida https://github.com/vacuumlabs/immutable-utils/blob/master/src/functions.js is separate thing. They are not needed for the PR, I just think it is convenient to have function equivalents for Immutable methods, but it is separate issue and I can separate this to distinct repositories if you like.

If we discard functions.js, you can think about the resulting code as a refactor of State class.