Refactoring a JavaScript class

This article is taken from the book JavaScript for Rails Developers (use ONE-YEAR-OLD to get 25% discount; valid until 2025-04-09 🥳). It is a book I published about a year ago. Over that period, many hundreds bought the book. It is written for Ruby/Rails developers to make JavaScript your 2nd favourite language.

I always get a little excited when I see a good refactoring happen. So I want to share this article; it is one of the last chapters where I go over an exisintg part of the code that is created in the book to refactor it with the goal to make it:

more readable;
easier to understand at a glance.

This is the current code it started with:

import { Annotation, Transaction } from "@codemirror/state"
import { EditorView } from "codemirror"

const editorCache = {
  pairs: new WeakMap(),
  store(sourceEditor, duplicateEditor) {
    this.pairs.set(sourceEditor, duplicateEditor)
  },

  get(editor) {
    return this.pairs.get(editor)
  }
}

export const splitView = {
  for: (editor, { enabled }) => {
    if (!enabled) return

    const syncAnnotation = Annotation.define()
    const sync = (transaction, sourceEditor) => {
      const documentLengthMismatch = transaction.changes && transaction.changes.length !== sourceEditor.state.doc.length

      if (documentLengthMismatch) return

      sourceEditor.update([transaction])

      const hasContentChanges = !transaction.changes.empty
      const isUserChange = !transaction.annotation(syncAnnotation)

      if (hasContentChanges && isUserChange) {
        const targetEditor = editorCache.get(sourceEditor)

        if (!targetEditor) return

        const annotations = [
          syncAnnotation.of(true),
          transaction.annotation(Transaction.userEvent)
        ].filter(Boolean)

        targetEditor.dispatch({
          changes: transaction.changes,
          annotations
        })
      }
    }

    const newEditor = new EditorView({
      state: editor.state,
      parent: editor.dom.parentElement,
      dispatch: transaction => sync(transaction, newEditor)
    })

    const duplicateEditor = new EditorView({
      state: newEditor.state,
      parent: editor.dom.parentElement,
      dispatch: transaction => sync(transaction, duplicateEditor)
    })

    editorCache.store(newEditor, duplicateEditor)
    editorCache.store(duplicateEditor, newEditor)

    editor.destroy()

    return duplicateEditor
  },

  destroy: (editor) => {
    editor?.destroy()
  }
}

It takes a lot brainpower to understand or just scan what is happening and how things flow. I can see a named export for splitView that has two methods: for and destroy. And only destroy is instantly clear to me. That whole for method takes some serious computing time. By the time, a few weeks later, you need to make changes again to this file (or someone else from your team), you already have forgotten what the flow of the code was.

So here is my proposed, refactored class. Below it I will highlight some of the details.

import { Annotation, Transaction } from "@codemirror/state"
import { EditorView } from "codemirror"

class SplitViewClass {
  create(editor, enabled) {
    if (!enabled) return

    const firstEditor = this.#createClone({ from: editor })
    const secondEditor = this.#createClone({ from: editor })

    this.#pair(firstEditor, { with: secondEditor })

    editor.destroy()

    return secondEditor
  }

  // private

  #pairs = new WeakMap()
  #syncAnnotation = Annotation.define()

  #createClone({ from: sourceEditor }) {
    return new EditorView({
      state: sourceEditor.state,
      parent: sourceEditor.dom.parentElement,
      dispatch: (transaction, editor) => this.#sync(transaction, { to: editor })
    })
  }

  #pair(firstEditor, { with: secondEditor }) {
    this.#pairs.set(firstEditor, secondEditor)
    this.#pairs.set(secondEditor, firstEditor)
  }

  #sync(transaction, { to: firstEditor }) {
    if (this.#cannotSync(transaction, { to: firstEditor })) return

    firstEditor.update([transaction])

    if (!transaction.changes.empty && !transaction.annotation(this.#syncAnnotation)) {
      const secondEditor = this.#pairs.get(firstEditor)

      if (!secondEditor) return

      secondEditor.dispatch({
        changes: transaction.changes,
        annotations: this.#createAnnotations({ from: transaction })
      })
    }
  }

  #cannotSync(transaction, { to: firstEditor }) {
		return transaction.changes?.length !== firstEditor.state.doc.length
  }

  #createAnnotations({ from: transaction }) {
    return [
      this.#syncAnnotation.of(true),
      transaction.annotation(Transaction.userEvent)
    ].filter(Boolean)
  }
}

export const splitView = {
  for: (editor, { enabled }) => new SplitViewClass().create(editor, { enabled }),
  destroy: (editor) => editor?.destroy()
}

In terms of lines of code, they are very much the same (70 vs 71; refactored is longer!). But I argue the refactored version is way easier to grasp. Just one public method, create, that is readable.

create(editor, enabled) {
  if (!enabled) return

  const firstEditor = this.#createClone({ from: editor })
  const secondEditor = this.#createClone({ from: editor })

  this.#pair(firstEditor, { with: secondEditor })

  editor.destroy()

  return secondEditor
}

It is actual readable: return when not enabled. Then create one cloned editor from the (given) editor, then another one stored as secondEditor. Then pair the first editor with the second editor.

From here you can follow the code as as you need more details as the methods are ordered by level of abstraction: createClone, pair. And then methods invoked from there. Using named parameters like { from: ... } and { with: ... } really helps with making the intent clear.

Look at this early return:

if (this.#cannotSync(transaction, { to: firstEditor })) return

Chef’s kiss, am I right?

Also small, focused methods that don’t give you a headache! Each method does one thing, uses early returns for clarity so no complex conditionals. And what’s more, as done earlier in chapter 8.2, the API stays the same. So to use this class with the bundled resource, simply replace: import { splitView } from "./editor/splitView" with import { splitView } from "./editor/splitViewClass".

destroy just calls the editor’s built-in destroy() method (using optional chaining ?. (as covered in an earlier chapter) in case the editor is undefined), without needing to handle any cleanup of our pairs since the WeakMap automatically handles that when editors are garbage collected.

A previous refactoring article from the book can be read here.

Over to you…

What did you like about this article? Learned something knew? Found something is missing or even broken? 🫣 Let me (and others) know!

Comments are powered by Chirp Form

Refactoring a JavaScript class

Want to read me more?