Initializing events on class

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])

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.

StackExchange Code Review Q#51729, answer score: 2