Coffeescript beautification and refactoring

Question

As much as I try, I cannot seem to get this Coffeescript code to look beautiful (I'd like to think it is possible). I have tried both Javascript and Coffeescript. Just to be clear, this code works fine, but it is hard to read, for reasons that I am unable to pinpoint.

How can it be refactored, reorganized, and what changes to coding style can be made to make it more appealing to read?

define [
  "plugins/ui/ui",
  "./js/highlight"
], (ui, highlight) ->
  editor = {}

  jQuery ($) ->
    # A widget to view source code.
    $.widget 'core.editor',
      _create: ->
        $editor = $(this.element)
        $editor
          .addClass('editor')
          .append($('<ul spellcheck="false" contenteditable> <li></li> </ul>'))

        # Move the gutter along with the editable area.
        this.lines().bind 'scroll', (event) ->
          $this = $(this)
          $this.siblings(".gutter").css(top: $this.scrollTop() * -1)

      # Highlight the sourceview using the given language.
      # The language's JSON rule file is loaded.
      highlight: (language) ->
        this.language = language
        require ["text!plugins/editor/js/#{ language }.json"], (json) =>
          this._rules = JSON.parse(json)
          return

      # Update the `left` of the `<ul>` based on the gutter width.
      # Each time the number of digits in the gutter changes, it becomes wider or
      # narrower, and the editor needs the shift accordingly.
      updateGutterWidth: () ->
        # The `8` is the gutter's left padding.
        this.lines().css(left: this.gutter().width() + 8)

      # Add or remove line numbers if the number of lines has changed.
      # `change` is a modification the the line count (In case the character was not yet
      # typed).
      updateLineNumbers: (change = 0) ->
        $gutter = this.gutter()
        count =   this.lines().children("li").length
        current = $gutter.children("span").length

        count += change

        # Add lines
        if (count > current)
          for i in [current..(count - 1)]
            ele = document.createElement("span")
            ele.innerText = "#{ i + 1 }"
            $gutter[0].appendChild(ele)
        # Remove lines
        else if (current > count)
          for j in [count..(current - 1)]
            $gutter.children("span:last-child").remove()

        this.updateGutterWidth() if current != count
        return

      # Set whether or not the gutter should be visible.
      lineNumbers: (bool) ->
        if bool == true and !this.number
          $(this.element)
            .prepend('<div class="gutter"></div>')
          this.lines()
            .css(left: 20)
          this.updateLineNumbers()
        else if bool == false and this.number
          this.gutter().remove()
          $(this.element)
            .css(left: 1)
        this.number = bool

      # Return the gutter (a jQuery object).
      gutter: () ->
        this._gutter ?= $(this.element).children("div.gutter")
        return this._gutter

      # Return a jQuery `<ul>`. Each `<li>` is a line of the source viewer.
      lines: ->
        return $(this.element).children('ul')

      # A hash of syntax highlighting rules.
      rules: ->
        return this._rules

    # Re-highlight the text.
    $(".editor > ul").live 'keyup', (event) ->
      # 13:              Enter
      # 37, 38, 39, 40:  Arrow Keys
      # 33, 34:          Page up / down
      # 16, 17, 18, 91:  Shift, Ctrl, Alt, Meta
      # 35, 36:          Home / end
      if !(event.which in [13, 37, 38, 39, 40, 33, 34, 16, 17, 18, 91, 35, 36]) and !event.altKey and !event.ctrlKey
        # Prevent an annoying error when backspacing to the beginning of a line.
        selection = window.getSelection()
        # Store the cursor position before highlighting.
        cursorPos = selection.getRangeAt(0)
        if cursorPos.getClientRects()[0]
          clickx = cursorPos.getClientRects()[0].left
          clicky = cursorPos.getClientRects()[0].top
          # Highlight
          $li = $(selection.focusNode).closest("li")
          rules = $li.closest(".editor").editor('rules')
          highlight.highlight($li, rules)
          # Restore cursor position.
          cursorPos = document.caretRangeFromPoint(clickx, clicky)
          window.getSelection().addRange(cursorPos)

    # Line numbering update.
    $(".editor > ul").live 'keydown', (event) ->
      # Redo line numbering for Enter, Backspace, Delete.
      if (event.which in [13, 8, 46])
        $this = $(this)
        newline = switch event.which
          when 13 then 1
          when 8  then -1
          else 0
        $this.parent().editor('updateLineNumbers', newline)
        # Correction
        setTimeout(() ->
          $this.parent().editor('updateLineNumbers', 0)
        , 300)




    # ##################### MAIN ##########################
    $(".frame").frame('tabs').last().tab("content")
      .append("<div id='sourceview'></div>")
    $("#sourceview")
      .css
        position: 'absolute'
        left:   1
        right:  1
        top:    1
        bottom: 1
      .editor()
      .editor('theme', 'plugins/editor/themes/idlefingers.css')
      .editor("highlight", "javascript")
      .editor("lineNumbers", true)


  return editor

Julian Birch · Accepted Answer · 2011-07-07 18:47:21Z

Yeah, coffeescript doesn't really improve method chaining, of which jQuery is so fond. A couple of tricks. First, remember how powerful destructuring is

e.g.

    if cursorPos.getClientRects()[0]
      clickx = cursorPos.getClientRects()[0].left
      clicky = cursorPos.getClientRects()[0].top

could be

if r = cursorPos.getClientRects()[0]
    {left, top} = r

also bear in mind

 @gutter

is the same as

 this.gutter

and

 highlight: (language) ->
    this.language = language

could be written

 highlight: (@language) ->

Finally, consider the use of closures for something you keep doing, unless performance is really critical.

e.g.

$this.parent().editor('updateLineNumbers', newline)
setTimeout(() ->
    $this.parent().editor('updateLineNumbers', 0)
    , 300)

could be

updateLineNumbers = (p) -> $this.parent().editor 'updateLineNumbers', p
updateLineNumbers newLine
after 300, () -> updateLineNumbers 0

cube242 · Accepted Answer · 2011-05-06 09:59:42Z

12

One more thing. I almost always rewrite my own version of setTimeout and setInterval when using CoffeeScript, so I can use a cleaner syntax when callbacks are the last argument.


after = (ms, cb) -> setTimeout cb, ms
every = (ms, cb) -> setInterval cb, ms


// that way, instead of this
setTimeout(() ->
  doSomething()
  doMore()
), 100)


// you can use this
after 100, () ->
  doSomething()
  doMore()

answered May 6, 2011 at 9:59

cube242

1212 bronze badges

Add a comment |

Adam · Accepted Answer · 2011-04-14 07:11:38Z

Here are some simple suggestions:

Put some spaces in between lines of code. Some whitespace will make things easier to read
I noticed you are using two spaces for indentation. In general, four spaces (or a tab) makes for more readable code
Don't break lines on chained methods unless you have a ton of them. For example, $(this.element).css(left: 1) can all go on one line no problem. If you have more than 2 or 3, placing each chained method on a new line is a good idea.
Easy on the comments. Commenting your code is good, but too much writing and it inhibits the readability of your code. If you have to write a lot of stuff, put it in your documentation instad. For example, you shouldn't include the key bindings of all those numbers.
Rather than pass a long string as an argument to a function, store it in a local variable on the preceding line, then pass the variable. Instead of:

$(this).html('bigass html string');

do this:

htmlStr = 'bigass html str";

$(this).html(htmlStr);
Take advantage of coffeescript's array syntax

keys = [
    13
    37, 38, 39, 40
    33, 34
    16, 17, 18, 91
    35, 36
]
Use @property, which coffeescript makes available as shorthand for this.property
Try programming with classes, and keeping as much of your jQuery event bindings in a separate area of your code. That way you can keep all of the $'s and other jQuery junk out of the code that actually makes your app work. Organizing your code into a structured class hierarchy will introduce some much-needed modularity to your coding. Breaking up chunks of commonly used code into reusable modules will make things easier to read and encourage best practices.

Stack Exchange Network

Coffeescript beautification and refactoring

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Coffeescript beautification and refactoring

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions