assoc-in about squint

[lilactown]

Bringing this discussion from #17 here @borkdude @corasaurus-hex

@borkdude wrote:

if assoc-in should make clones of each object, or update them in place. This is an important decision that needs some thought instead of slipping it through in a PR I think?

I think we should have both assoc-in! that would update each object in place, and assoc-in that would copy each object.

For assoc-in, the type of use case I'm thinking of is writing deeply nested immutable changes to state in UI applications. My experience with Redux and other UI state management systems (even just plain React) is that this is desirable. See the example here: https://redux.js.org/tutorials/fundamentals/part-3-state-actions-reducers#handling-additional-actions

export default function appReducer(state = initialState, action) {
  switch (action.type) {
    case 'todos/todoAdded': {
      return {
        ...state,
        todos: [
          ...state.todos,
          {
            id: nextTodoId(state.todos),
            text: action.payload,
            completed: false
          }
        ]
      }
    }
    case 'todos/todoToggled': {
      return {
        ...state,
        todos: state.todos.map(todo => {
          if (todo.id !== action.payload) {
            return todo
          }

          return {
            ...todo,
            completed: !todo.completed
          }
        })
      }
    }
    case 'filters/statusFilterChanged': {
      return {
        // Copy the whole state
        ...state,
        // Overwrite the filters value
        filters: {
          // copy the other filter fields
          ...state.filters,
          // And replace the status field with the new value
          status: action.payload
        }
      }
    }
    default:
      return state
  }
}

This can be written using immutable update, assoc & assoc-in very nicely:

(defn app-reducer [state action]
  (case (:type action)
    :todos/todo-added
    (update state :todos conj {:id (next-todo-id state)
                               :text (:payload action)
                               :completed false})
    :todos/todo-toggles
    (assoc state :todos
           (map #(if (= (:id %) (:payload action))
                   (update % :completed not)
                   %)
                (:todos state)))
    :todos/status-filter-change
    (assoc-in state [:filters :status] (:payload action))))

[borkdude]

Agreed: assoc-in should be fully "shallow" immutable and assoc-in! can be in-place updating. This would seem the least surprising behavior. PRs welcome for each.

[corasaurus-hex]

I guess I should split up my open PR then 😅

[borkdude]

@corasaurus-hex If you could change your current implementations to align with the above, for now I'm fine with merging your current PR and then in the future, let's work in smaller chunks.

[corasaurus-hex]

luckily they already work this way so I don't need to make any changes 😄

[corasaurus-hex]

maybe I should change the tests for assoc-in to demonstrate that they're not the same objects, like how for assoc-in! I demonstrate that they are the same objects?

[borkdude]

Good idea!

[borkdude]

I merged your PR, feel free to provide incremental PRs with tests.