Initializing events on class

Question

I wrote a simple class that substitutes all file inputs in an HTML for a more complex template and set some actions on it. The code is rather small, but I'm not used to write JavaScript.

I wonder if there is a design pattern or extensively used way to set the callbacks? Of course, overall improvements are welcome as well.

The full code is here:

$(document).ready ->
  $('input[type="file"]').each ->
    new PhotoInput(this)

class PhotoInput
  constructor: (originalInput) ->
    @renderTemplate(originalInput)
    @setClickActions()
    @setPreviewRender()

  renderTemplate: (originalInput) ->
    @template = ich.image_file_input_template # this is ICanHazJS call - it loads a mustache template there.
      fileInput: $(originalInput)[0].outerHTML
    @fileInput = @template.find('input[type=file]').first()
    @previewImage = @template.find('.file-input-preview img')
    $(originalInput).replaceWith(@template)

  setClickActions: ->
    @template.find('#select-file').click =>
      @fileInput.focus().click()
    @template.find('.file-input-preview').click =>
      @fileInput.focus().click()

  setPreviewRender: ->
    @fileInput.change =>
      @previewImage.attr('alt', @fileInput.val())
      if(@fileInput[0].files && @fileInput[0].files[0])
        reader = new FileReader()
        reader.onload = (e) =>
          @previewImage.attr('src', e.target.result)
        reader.readAsDataURL(@fileInput[0].files[0])

Answer 1

You're instantiating a PhotoInput object but never assigning it to a variable. This implies that there's no need for a class at all. You're only instantiating it for its side-effects; you're not interested in the instance itself.

This works, but it's bad form.

Even if you stored the instance somewhere, you still can't really use it for anything. Sure, you could call its methods, like, say, setPreviewRender, but there'd be no point since it's already been called - in fact, it'd probably be a bad idea to call it again.

So don't use a class, just write a function and use closures instead of instance variables:

createPhotoInput = (originalInput) ->
  originalInput = $ originalInput

  # replace original input
  template = ich.image_file_input_template
    fileInput: originalInput.get(0).outerHTML

  originalInput.eq(0).replaceWith template

  # set click handlers
  input = template.find 'input[type=file]:first'
  template.find('#select-file, .file-input-preview').click (event) ->
    input.focus().click()

  # set preview handling
  preview = template.find '.file-input-preview img'
  input.change (event) ->
    preview.attr 'alt', input.val()
    file = input.get(0).files?[0]
    if file?
      reader = new FileReader
      reader.onload = (event) -> previewImage.attr 'src', e.target.result
      result.readAsDataURL file

  # return the template, just in case
  template

$ ->
  $('input[type="file"]').each ->
    createPhotoInput this

You'll note that it also does away with all the scope-handling and fat arrows (=>), since it's just closures.

Answer 2

I think a class is fine to use here. I think it's more readable and easier to test.

Though I reserve my right to change my answer after I get some feedback on a related question, I like to initialize all instance variables and event handling in the constructor so it's easy to see at a glance what the variables and events are:

class PhotoInput
  constructor: (originalInput) ->  
    # Setup all instance variables
    @template = renderTemplate(originalInput)
    @fileSelect = @template.find('#select-file')
    @fileInputPreview = @template.find('.file-input-preview')
    @previewImage = @template.find('.file-input-preview img')
    @fileInput = @template.find('input[type=file]').first()

    # Setup events
    @fileSelect.click(@focusFileInput)
    @fileInputPreview.click(@focusFileInput)
    @fileInput.change(@setPreviewRender)

  focusFileInput: =>
    @fileInput.focus().click()

  # this is ICanHazJS call - it loads a mustache template there.
  renderTemplate: (originalInput) ->
    template = ich.image_file_input_template(fileInput: $(originalInput)[0].outerHTML)
    $(originalInput).replaceWith(@template)
    template

  setPreviewRender: =>
    @previewImage.attr('alt', @fileInput.val())
    if(@fileInput[0].files && @fileInput[0].files[0])
      reader = new FileReader()
      reader.onload = (e) => @previewImage.attr('src', e.target.result)
      reader.readAsDataURL(@fileInput[0].files[0])

$ ->
  $('input[type="file"]').each ->
    new PhotoInput(this)