3
\$\begingroup\$

So, I wrote this small library for fast DOM building with API tailored for CoffeeScript

I feel like the code overall can be made better/smaller/faster.

I am also not particularly happy about a few functions, namely:

  # Parses out css classes and id from string like:
  #   p#warning.big.yellow # => p   # attr {"id": "warning", "class": ['big', 'yellow']}
  #   #container           # => div # attr {"id": "container"}
  # @returns node name (e.g. "span")
  dotHashRe = /[.#]/
  parseElem = (elem, attr) ->
    return elem unless dotHashRe.test elem
    attr['class'] ||= []
    attr['class'] = [attr['class']] if typeof attr['class'] == 'string'
    elem     = "div#{elem}" if dotHashRe.test(elem.charAt(0))
    pieces   = elem.split(dotHashRe)
    elemType = pieces.shift()
    pos      = elemType.length
    classes  = attr['class']
    for piece in pieces
      if elem.charAt(pos) == '#'
        attr['id'] = piece
      else 
        classes.push(piece)
      pos += piece.length + 1
    delete attr['class'] unless attr['class'].length
    elemType

It is fast, but it looks like the code can be made a lot nicer

The library (~100 LOC with comments): https://github.com/glebm/DOMBrew/blob/master/dombrew.js.coffee

Looking forward to the suggestions! :)

\$\endgroup\$
3
\$\begingroup\$

A bunch of random observations. I'd rethink the way you approach "classes". I'd start with something like

classes = attr['class'] || []
classes = [classes] if typeof classes == 'string'

And then write or delete it back at the end.

Creating hashStartRe

hashStartRe = /^[.#]/
elemType = if hashStartRe.test elem then "div" else pieces.shift()

Will probably be faster than the current rewriting and calling elem.charAt(0). It's definitely easier to read. However, it causes problems with "pos". (Solveable problems, but you do need to be careful.)

At this point, the big question is whether you want to allow multiple ids or ids that aren't in the front position. If you restrict your attention to a#id.class1.class2 and don't process a.class1#id1#id2, you can simplify the for loop as well.

A final performance point, since regular expressions are so fast on modern browsers, you may want to consider using the regular expression /(.#)([^.#)+)/ to do your parsing. But you'd need to sit down and do some proper analysis on what was faster. My gut tells me it will be.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.